separate thread_init/term from gc_init/term #2079
Conversation
MartinNowak
commented
Feb 7, 2018
MartinNowak
commented
- make thread_init/term @nogc
- move thread_init/term from GC to rt_init/term
- needed to replace destroy with manual destruction - use custom monitor deletion that does not call any event hooks
|
Thanks for your pull request, @MartinNowak! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
MartinNowak
requested review from
Burgos,
DmitryOlshansky,
PetarKirov,
andralex,
rainers and
wilzbach
as code owners
Let's make it pass -> #2080 |
schveiguy
left a comment
schveiguy
left a comment
There was a problem hiding this comment.
Nice! I think this is a much more straightforward solution.
| Thread.sm_main.__dtor(); | ||
| _d_monitordelete_nogc(Thread.sm_main); | ||
| if (typeid(Thread).initializer.ptr) | ||
| _mainThreadStore[] = typeid(Thread).initializer[]; |
There was a problem hiding this comment.
A couple comments here:
My coverage analysis is saying this code is never run.
The normal destroy will nullify the vtable, and also if the initializer.ptr is null, it means you should zero the data. But I don't actually think you need to check if it's null, as it should always have a vtable in the initializer.
There was a problem hiding this comment.
Coverage messurement fails here because this is run outside of the main loop, i.e. after coverage is collected and reported.
There was a problem hiding this comment.
BTW, you can easily fixup or edit PRs. Much less hassle than asking for single-line changes.
andralex
left a comment
andralex
left a comment
There was a problem hiding this comment.
This is superb work. Thanks!
|
Squash please. Then I will adjust my PR. Thanks! |
|
Coverage needs to be looked at but I'll merge this to unblock @somzzz who is in a bit of time crunch. |
|
No need to squash here, these are 3 separate steps (commits) that together form one particular change. |
6 participants
