Fixed a problem with spurious wakeups.

runtime/vm/thread.h

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-#ifndef VM_THREAD_H_
-#define VM_THREAD_H_
+#ifndef RUNTIME_VM_THREAD_H_
+#define RUNTIME_VM_THREAD_H_

[siva, 2011/10/21 21:18:33]

Why did you change this? If you look at all the other header files you will see
a uniform pattern and we need to stick to that.

[jrgfogh, 2011/10/24 09:20:42]

A presubmit script told me to do it when I tried to upload the CL. It was based
on some version of lint.

[Søren Gjesse, 2011/10/26 12:07:10]

That is annoying. When this error is the only one the presubmit script gives run
it wiht --bypass_hooks to upload anyway.

 
 #include "vm/assert.h"
 #include "vm/allocation.h"
 #include "vm/globals.h"
 
 // Declare the OS-specific types ahead of defining the generic classes.
 #if defined(TARGET_OS_LINUX)
 #include "vm/thread_linux.h"
 #elif defined(TARGET_OS_MACOS)
 #include "vm/thread_macos.h"
@@ skipping 46 matching lines @@
 
   static const int64_t kNoTimeout = 0;
 
   Monitor();
   ~Monitor();
 
   void Enter();
   void Exit();
 
   // Wait for notification or timeout.
+  // May return too early due to spurious wakeups. Callers should test for this.

[siva, 2011/10/21 21:18:33]

I think this comment is not necessary.

[jrgfogh, 2011/10/24 09:20:42]

I disagree, since it affects the semantics. I would normally expect a class
Monitor to have correct monitor semantics, which this one doesn't.

   WaitResult Wait(int64_t millis);
 
   // Notify waiting threads.
   void Notify();
   void NotifyAll();
 
  private:
   MonitorData data_;  // OS-specific data.
 
   DISALLOW_COPY_AND_ASSIGN(Monitor);
@@ skipping 28 matching lines @@
     ASSERT(monitor != NULL);
     // TODO(iposva): Consider adding a no GC scope here.
     monitor_->Enter();
   }
 
   virtual ~MonitorLocker() {
     monitor_->Exit();
     // TODO(iposva): Consider decrementing the no GC scope here.
   }
 
+  // May return too early due to spurious wakeups. Callers should test for this.

[siva, 2011/10/21 21:18:33]

Ditto.

[jrgfogh, 2011/10/24 09:20:42]

Ditto.

   Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout) {
     return monitor_->Wait(millis);
   }
 
   void Notify() {
     monitor_->Notify();
   }
 
   void NotifyAll() {
     monitor_->NotifyAll();
   }
 
  private:
   Monitor* const monitor_;
 
   DISALLOW_COPY_AND_ASSIGN(MonitorLocker);
 };
 
 }  // namespace dart
 
 
-#endif  // VM_THREAD_H_
+#endif  // RUNTIME_VM_THREAD_H_

[siva, 2011/10/21 21:18:33]

ditto.

[jrgfogh, 2011/10/24 09:20:42]

ditto.