From b81a643f9ae341b0c23cecc54daccdc8d7bc746a Mon Sep 17 00:00:00 2001 From: Dmitri Melikyan Date: Thu, 14 May 2015 12:39:16 +0200 Subject: V8: avoid deadlock when profiling is active A deadlock happens when sampler initiated by SIGPROF tries to lock the thread and the thread is already locked by the same thread. As a result, other thread involved in sampling process hangs. The patch adds a check for thread lock before continuing sampler operation. The fix has been tested on a sample app under load with and without profiling turned on. Fixes issue #14576 and specifically the duplicate issue #25295 Reviewed-By: Julien Gilli PR-URL: https://github.com/joyent/node/pull/25309 --- deps/v8/src/isolate.h | 5 ++++ deps/v8/src/sampler.cc | 8 +++++ test/simple/test-regress-GH-14576.js | 57 ++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 test/simple/test-regress-GH-14576.js diff --git a/deps/v8/src/isolate.h b/deps/v8/src/isolate.h index 9ef6fc732..eae5cad28 100644 --- a/deps/v8/src/isolate.h +++ b/deps/v8/src/isolate.h @@ -384,6 +384,11 @@ class Isolate { public: ~Isolate(); + // ISSUE-14576: allow to access process_wide_mutex_ from sampler.cc + static base::Mutex* GetProcessWideMutexPointer() { + return process_wide_mutex_.Pointer(); + } + // A thread has a PerIsolateThreadData instance for each isolate that it has // entered. That instance is allocated when the isolate is initially entered // and reused on subsequent entries. diff --git a/deps/v8/src/sampler.cc b/deps/v8/src/sampler.cc index 0be31b51b..fba8401f5 100644 --- a/deps/v8/src/sampler.cc +++ b/deps/v8/src/sampler.cc @@ -342,6 +342,14 @@ void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info, // We require a fully initialized and entered isolate. return; } + + // ISSUE-14576: To avoid deadlock, return if there is a thread lock + if (Isolate::GetProcessWideMutexPointer()->TryLock()) { + Isolate::GetProcessWideMutexPointer()->Unlock(); + } else { + return; + } + if (v8::Locker::IsActive() && !isolate->thread_manager()->IsLockedByCurrentThread()) { return; diff --git a/test/simple/test-regress-GH-14576.js b/test/simple/test-regress-GH-14576.js new file mode 100644 index 000000000..40839914b --- /dev/null +++ b/test/simple/test-regress-GH-14576.js @@ -0,0 +1,57 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +/* + * Tests for sampler deadlock regression. + * Issue https://github.com/joyent/node/issues/14576 + */ + +var assert = require('assert'); +var cp = require('child_process'); + +var child = undefined; + +if (process.platform !== 'win32') { + process.on('SIGTERM', function() { + if(child) { + process.kill(child.pid, 'SIGKILL'); + } + + process.exit(); + }); +} + +var testScript = "var i = 0; function r() { if(++i > 25) return; " + + "setTimeout(r, 1); }; r();" +var nodeCmd = process.execPath + + ' --prof --nologfile_per_isolate' + + ' -e "' + testScript + '"'; +var runs = 0; + +function runTestScript() { + child = cp.exec(nodeCmd, function(err, stdout, stderr) { + if (++runs > 50) return; + + runTestScript(); + }); +} + +runTestScript(); -- cgit v1.2.1