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

We do all this bookkeeping about who owns what lock, but that is all redundant
now that we use `PTHREAD_MUTEX_ERRORCHECK` with `js::Mutex` in `DEBUG` builds.

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

The checks that the `lockOwner` member was used for are redundant now that
`js::Mutex` uses `PTHREAD_MUTEX_ERRORCHECK` in `DEBUG` builds.

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

The checks that the `lockOwner` member was used for are now redundant given that
`js::Mutex` uses `PTHREAD_MUTEX_ERRORCHECK` in `DEBUG` builds.

In methods where we asserted `isLocked()`, we now pass in an
`AutoLockHelperThreadState` reference for compile-time proof of lock
holding. This is mostly a mechanical change.

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

The checks that the `lockOwner` member was used for are redundant now that
`js::Mutex` uses `PTHREAD_MUTEX_ERRORCHECK` in `DEBUG` builds.

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

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

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

Comment on attachment 8775620 [details] [diff] [review]
Remove the DEBUG-only lockOwner member from GlobalHelperThreadState

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

::: js/src/vm/Runtime.cpp
@@ +908,3 @@
>          MOZ_FALLTHROUGH;
>        case GCLock:
>          break;

It would be nice to find a way to assert ordering of these locks statically. Maybe AutoLockGC should take the HelperThreadLock in its constructor?

	Pulsebot
	Comment 6 • 8 years ago

Pushed by nfitzgerald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/573529c879ad
Remove the GCRuntime DEBUG-only lockOwner field; r=terrence
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bc0a14de00a
Remove the DEBUG-only lockOwner member from GlobalHelperThreadState; r=terrence
https://hg.mozilla.org/integration/mozilla-inbound/rev/41096329fbed
Remove the DEBUG-only lockOwner member from vixl::Simulator; r=terrence

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 7 • 8 years ago
backout bugherder landing

This or something else from the push appears to have turned 10.10 debug cpp tests permared: https://treeherder.mozilla.org/logviewer.html#?job_id=33028238&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/8786fe9a1993

	Pulsebot
	Comment 8 • 8 years ago

Pushed by nfitzgerald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a261bbbe735
Remove the GCRuntime DEBUG-only lockOwner field; r=terrence
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9bc531fdc75
Remove the DEBUG-only lockOwner member from GlobalHelperThreadState; r=terrence
https://hg.mozilla.org/integration/mozilla-inbound/rev/3335ffe05307
Remove the DEBUG-only lockOwner member from vixl::Simulator; r=terrence

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

https://hg.mozilla.org/mozilla-central/rev/4a261bbbe735
https://hg.mozilla.org/mozilla-central/rev/a9bc531fdc75
https://hg.mozilla.org/mozilla-central/rev/3335ffe05307