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

Issue 8347013: Fix more races in MessageQueue_WaitNotify. My notifies could show up (Closed)

Created:
9 years, 2 months ago by turnidge
Modified:
9 years, 2 months ago
Reviewers:
siva
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Fix more races in MessageQueue_WaitNotify. My notifies could show up before my waits, and I would timeout. Committed: https://code.google.com/p/dart/source/detail?r=536

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -5 lines) Patch
M runtime/vm/message_queue_test.cc View 2 chunks +6 lines, -5 lines 3 comments Download

Messages

Total messages: 2 (0 generated)
turnidge
TBR=iposva
9 years, 2 months ago (2011-10-18 21:41:18 UTC) #1
siva
9 years, 2 months ago (2011-10-18 22:19:59 UTC) #2
DBC.

http://codereview.chromium.org/8347013/diff/1/runtime/vm/message_queue_test.cc
File runtime/vm/message_queue_test.cc (right):

http://codereview.chromium.org/8347013/diff/1/runtime/vm/message_queue_test.c...
runtime/vm/message_queue_test.cc:73: }
The update to shared_queue could be under a lock:
{
  MonitorLocker ml(sync);
  shared_queue = queue;
  ml.notify();
}

http://codereview.chromium.org/8347013/diff/1/runtime/vm/message_queue_test.c...
runtime/vm/message_queue_test.cc:79: }
The check for peer.HasMessage() could be under a lock,
Also we should probably be using a different monitor for
this as the notify could get eaten by the same thread that
did the notify.

{
  MonitorLocker ml(sync1);
  while (!peer.HasMessage()) {
    ml.Wait();
  }
}

http://codereview.chromium.org/8347013/diff/1/runtime/vm/message_queue_test.c...
runtime/vm/message_queue_test.cc:113: }
The check for shared queue could be under a lock:
{
  MonitorLocker ml(sync);
  while (shared_queue == NULL) {
    ml.Wait();
  }
}

Powered by Google App Engine
This is Rietveld 408576698