Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Assignee
	Description • 8 years ago

      No description provided.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Assignee
	Comment 2 • 8 years ago

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b6d112b3c92

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Assignee
	Comment 4 • 8 years ago

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9701df4719b5

	Terrence Cole [:terrence]
	Comment 5 • 8 years ago

Comment on attachment 8762740 [details] [diff] [review]
Use js::Mutex/js::ConditionVariable instead of PRLock/PRCondVar in js::Monitor

Review of attachment 8762740 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/Monitor.cpp
@@ -19,5 @@
> -    if (!condVar_)
> -        return false;
> -
> -    return true;
> -}

\o/

	Bert Belder
	Comment 6 • 8 years ago

Comment on attachment 8762740 [details] [diff] [review]
Use js::Mutex/js::ConditionVariable instead of PRLock/PRCondVar in js::Monitor

Review of attachment 8762740 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/threading/Mutex.h
@@ +40,5 @@
>      return *this;
>    }
>  
> +  bool operator==(const Mutex& rhs) {
> +    return platformData_ == rhs.platformData_;

This will probably work in practice, but it doesn't seem correct to me; there's no guarantee that the *contents* of platformData_ uniquely identifies the lock (consider an implementation that uses a spinlock - platformData_ may just be counter).

Wouldn't it be more obvious to do `return this == &rhs` or, if you insist, `return &platformData_ == &rhs.platformData_` ?

	Bert Belder
	Comment 7 • 8 years ago

> This will probably work in practice, but it doesn't seem correct to me; there's no guarantee that the *contents* of platformData_ uniquely identifies the lock (consider an implementation that uses a spinlock - platformData_ may just be counter).

I was being dumb here - platformData_ is an array, so it already compares pointers and not contents.

	Pulsebot
	Comment 8 • 8 years ago

Pushed by nfitzgerald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9338313016a
Use js::Mutex/js::ConditionVariable instead of PRLock/PRCondVar in js::Monitor; r=terrence

	Carsten Book [:Tomcat]
	Comment 9 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/c9338313016a