Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(137)

Issue 629533002: Fix deadlock that can occur while handling service messages at a breakpoint. (Closed)

Created:
6 years, 2 months ago by turnidge
Modified:
6 years, 1 month ago
Reviewers:
Cutch
CC:
reviews_dartlang.org, vm-dev_dartlang.org, Ivan Posva
Visibility:
Public.

Description

Fix deadlock that can occur while handling service messages at a breakpoint. Deadlock looks like: Regular Isolate -> paused at breakpoint in debug message loop -> holds debug message queue lock to get message notifications -> handles an service message -> waits for portmap lock to send response Service Isolate -> receives request for paused isolate -> holds portmap lock to send message -> runs custom message notifier to wake debug message loop -> waits for debug message queue lock I've solved this by releasing the debug message queue lock while handling service messages. This requires me to poll for new service messages after reacquiring the debug message queue lock to make sure I haven't dropped any notifications. It's a little weird that the embedder (runtime/bin) needs to be aware of the locking in the core vm (runtime/vm), but this seemed like the simplest fix for now. BUG= R=johnmccutchan@google.com Committed: https://code.google.com/p/dart/source/detail?r=41326

Patch Set 1 #

Total comments: 1

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -32 lines) Patch
M runtime/bin/dbg_message.cc View 1 chunk +12 lines, -2 lines 0 comments Download
M runtime/include/dart_api.h View 1 chunk +7 lines, -0 lines 0 comments Download
M runtime/vm/dart_api_impl.cc View 1 1 chunk +7 lines, -0 lines 0 comments Download
M runtime/vm/message.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M runtime/vm/message_handler.h View 1 chunk +4 lines, -0 lines 0 comments Download
M runtime/vm/message_handler.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M runtime/vm/message_handler_test.cc View 1 chunk +21 lines, -0 lines 0 comments Download
M runtime/vm/message_test.cc View 5 chunks +7 lines, -28 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
turnidge
6 years, 2 months ago (2014-10-03 21:35:17 UTC) #2
turnidge
6 years, 2 months ago (2014-10-03 21:35:44 UTC) #3
Cutch
lgtm with comment https://codereview.chromium.org/629533002/diff/1/runtime/vm/message_handler.h File runtime/vm/message_handler.h (right): https://codereview.chromium.org/629533002/diff/1/runtime/vm/message_handler.h#newcode147 runtime/vm/message_handler.h:147: friend class MessageHandlerTestPeer; Remove this
6 years, 2 months ago (2014-10-03 21:43:07 UTC) #4
turnidge
6 years, 1 month ago (2014-10-27 16:57:29 UTC) #5
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 41326 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698