RFR: JDK-8293346: Add NoThreadCurrentMark to simulate non-attached threads [v2]

Thomas Stuefe stuefe at openjdk.org
Thu Nov 3 05:00:31 UTC 2022


On Thu, 22 Sep 2022 17:08:02 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> When working on [JDK-8293344](https://bugs.openjdk.org/browse/JDK-8293344), I needed a way to verify that I had removed all offending usages of ResourceArea memory inside an extend.
>> 
>> For that, a NoThreadCurrentMark was useful. It temporarily sets the current Thread* to NULL for the extend. Any use of Thread::current below that mark will now crash or assert.
>> 
>> This can be used for two things:
>> - guard code that is supposed to be safe for non-attached threads (os::malloc, stack printing, UL etc) against accidental usage of Thread::current() (e.g. resource area allocation)
>> - in gtests, simulate a non-attached thread to cover code that behaves differently with Thread::current()==NULL.
>> 
>> This patch:
>> 
>> - Introduces the debug-only `NoThreadCurrentMark`
>> - Adds a gtest for it
>> - Adds it to guard dwarf-parsing-based stack printing, which had not been safe due to use of ResourceArea, fixed with  [JDK-8293344](https://bugs.openjdk.org/browse/JDK-8293344)
>> - Adds it to `os::malloc()`, `os::realloc()` and `os::free()`, since those can certainly be called without a current thread.
>> - Replaces the test-local mark in the SafeFetch gtests that did a similar thing
>> - I may add and fix up other users later
>> 
>> I also had to change Library-based TLS initialization such that it runs at C++ dynamic initialization time. That is because the `NoThreadCurrentMark` needs to set both Library-based TLS and C++ TLS slots, and having library-based TLS not available before VM initialization made the code too complex. This also means we can remove the explicit TLS initialization from create_vm.
>> 
>> Finally, while I was here, I also fixed the error printing in library based TLS (since the pthread_key_xxx functions don't modify errno).
>
> Thomas Stuefe has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - New solution
>  - Revert changes to TLS initialization

I withdraw this since I don't have time to work on it. I think the idea was sound, but ultimately its not that important.

-------------

PR: https://git.openjdk.org/jdk/pull/10178


More information about the hotspot-dev mailing list