Thread detach stability

[acehreli]

(Trying again after closing PR 1986.)

Apologies for three separate commits. Two of the commits were necessary to get the test program passing.

• Prevent returning dangling pointer from thread_attachThis

• Call all detach varieties after locking slock. (All other calls to Thread.remove() are done while holding slock.)

• Do not pthread_detach non-D threads i.e. the ones that are attached by the user. After all, we don't know and have any say on the lifetimes of such threads.

[dlang-bot]

Thanks for your pull request, @acehreli! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	18063	thread_attachThis returns dangling pointer

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

[andralex]

lgtm

[jacob-carlborg]

About the tests only failing on macOS 32bit, I haven’t looked at the changes yet but note that macOS 64Bit uses native TLS implementation while 32bit uses a custom implementation.

[acehreli]

I have to determine whether the new test application would fail without the changes to thread.d. Maybe it's just exposing existing issues.

Meanwhile, I'm curious about "macOS 64Bit uses native TLS implementation while 32bit uses a custom implementation". Where can I learn more about the differences?

[jacob-carlborg]

Meanwhile, I'm curious about "macOS 64Bit uses native TLS implementation while 32bit uses a custom implementation". Where can I learn more about the differences?

Mostly in the code in the compiler. I added some documentation about the 64bit implementation [1] [2]. You can read about the custom implementation here [3].

[1] https://github.com/dlang/dmd/blob/master/src/ddmd/backend/el.c#L1284-L1306
[2] https://github.com/dlang/dmd/blob/master/src/ddmd/toobj.d#L1245-L1272
[3] http://www.drdobbs.com/architecture-and-design/implementing-thread-local-storage-on-os/228701185

[wilzbach]

Removed auto-merge for now to avoid clogging the priority queue of the auto-tester.

[TurkeyMan]

I have encountered this. Why not merge?

[aliak00]

Maybe related to this too? https://issues.dlang.org/show_bug.cgi?id=20085

[WalterBright]

@acehreli can you please fix the merge issues?