Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test uv_barrier_t size in right block #35

Merged
merged 1 commit into from
Feb 12, 2024
Merged

test uv_barrier_t size in right block #35

merged 1 commit into from
Feb 12, 2024

Conversation

semarie
Copy link

@semarie semarie commented Feb 11, 2024

the local uv_barrier_t implementation is used: for AIX, OpenBSD and any system not defining PTHREAD_BARRIER_SERIAL_THREAD.

as the uv_barrier_t struct was modified and it isn't a pointer anymore on such systems, the check for uv_barrier_t size isn't right for them. so move the check inside the block implementing uv_barrier_t using pthread_barrier_t.

tested on OpenBSD, where the build failed due to the STATIC_ASSERT().

the local uv_barrier_t implementation is used: for AIX, OpenBSD and any system
not defining PTHREAD_BARRIER_SERIAL_THREAD.

as the uv_barrier_t struct was modified and it isn't a pointer anymore, the
check for uv_barrier_t size isn't right anymore for systems not using local
uv_barrier_t implementation. so move the check inside the block implementing
uv_barrier_t using pthread_barrier_t.

tested on OpenBSD, where the build failed due to the STATIC_ASSERT().
@semarie
Copy link
Author

semarie commented Feb 11, 2024

I opened this PR in JuliaLang repository, as the code changing uv_barrier_t is specific to the julia version of libuv (or I didn't find it upstream).

@vtjnash
Copy link
Member

vtjnash commented Feb 12, 2024

Ah, thanks. Sorry about that. This should also go to libuv against the master branch there, if you don't mind

@vtjnash vtjnash merged commit 344a3f5 into JuliaLang:julia-uv2-1.48.0 Feb 12, 2024
5 of 37 checks passed
@semarie semarie deleted the uv_barrier_t-internal branch February 12, 2024 05:31
@semarie
Copy link
Author

semarie commented Feb 12, 2024

It has been commited to upstream libuv too (see libuv#4311).
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants