Closed Bug 756709 Opened 11 years ago Closed 10 years ago

Make ThreadLocal::set infallible by crashing on failure


(Core :: MFBT, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)



(1 file)

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.
No longer depends on: 753119
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.
Attached patch PatchSplinter Review
Assignee: nobody → jwalden+bmo
Attachment #694963 - Flags: review?(Ms2ger)
Comment on attachment 694963 [details] [diff] [review]

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

If that's what we want to do, r=me.
Attachment #694963 - Flags: review?(Ms2ger) → review+
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.  :-\

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.
Target Milestone: --- → mozilla20
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.