756709 - Make ThreadLocal::set infallible by crashing on failure

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Jeff Walden [:Waldo]]

Thread-local storage is apparently at least sometimes allocated lazily (by necessity, given dynamic library loading).  Thus storing a thread-local variable is fallible.  Right now ThreadLocal::set returns a bool to indicate success or failure.  It seems pretty unlikely, however, that users have a sane way to handle that failure in general.  So we should probably make this infallible, and if a failure does happen, we should just crash.

Comment 1

[David Lawrence [:dkl]]

Over the weekend an issue occurred with the BMO database which resulted in duplication of dependencies. The dependency issue may have resulted in "Depends On" and "Blocks" values being removed while updating a bug. This issue should now be resolved, however dependencies may need to be manually restored to some bugs.

This bug had dependencies removed during the failure period and will need verification that the dependency removal(s) were intentional. Please help out by taking a look at this bug and adding anything back that was mistakenly removed.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 694963 [details] [diff] [review]
Patch

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

If that's what we want to do, r=me.

Comment 4

[Jeff Walden [:Waldo]]

It's unfortunate the various platform TLS APIs don't permit splitting up the fallible part of ThreadLocal::set from the infallible part (say, with a pthread_ensure(index) call or something).  If there were such a thing I'd be more than willing to smack a warn-unused-result on the fallible part and keep .set infallible, but wishes and wings and all that.  :-\

https://hg.mozilla.org/integration/mozilla-inbound/rev/6e5acaff8239

The biggest indictment of fallible set is probably that *nobody*'s checking for failure, not even the JS engine (which ordinarily you'd expect to be better about that), so trying to hold the current course is -- well, not lost, quite, but definitely looking like a losing cause.

Comment 5

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/6e5acaff8239