On Fri, 22 Jul 2022 16:45:55 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> This patch removes the use of std::thread from the `java.lang.foreign`
>> tests, and switches to the OS specific thread APIs, in order to change
>> things such as the stack size on some platforms where this is required in
>> the future (see the JBS issue).
>>
>> This is done by adding a small header-only library that exposes a function
>> to run code in freshly spawned threads (`run_async`).
>>
>> Testing: Running the affected tests on platforms that implement the linker.
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixes
Hi @JornVernee,
good job and thanks for doing it so quickly. Looks good, module the
GetLastError comment.
About the stack size and Alpine, I am fine with doing the Aĺpine-specific fix
in a follow-up, or you can do it as part of this change. Strictly speaking its
not part of this bug report to adjust the stack size.
If you fix the GetLastError issue, I don't need another look.
Thanks, Thomas
test/lib/native/testlib_threads.h line 50:
> 48: static void fatal(const char* message) {
> 49: perror(message);
> 50: exit(-1);
Won't work as intended for Windows APIs. I would print the result of
`GetLastError()` instead.
Alternatively I am fine fine with just omitting the error code, because I think
the old tests did not handle errors either. Or did we catch std::thread
exceptions somewhere?
test/lib/native/testlib_threads.h line 61:
> 59: helper->proc(helper->context);
> 60: return 0;
> 61: }
I'm curious, does this only exist because of the DWORD vs void* return value
size of the native thread functions? I wondered why you don't just pass the
test procedure to the thread start API directly.
About stdcall, does Oracle still build Windows 32bit?
-------------
Marked as reviewed by stuefe (Reviewer).
PR: https://git.openjdk.org/jdk/pull/9599
