Make ThreadLocal::set infallible by crashing on failure

RESOLVED FIXED in mozilla20

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

unspecified
mozilla20
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Posted patch PatchSplinter Review
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #694963 - Flags: review?(Ms2ger)
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.
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.  :-\

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.
Target Milestone: --- → mozilla20
https://hg.mozilla.org/mozilla-central/rev/6e5acaff8239
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.