Skip to content

Fix endianness issue which fixes deadlock#88

Open
pranavkaruvally wants to merge 1 commit intogoogle:mainfrom
pranavkaruvally:s390x-fix
Open

Fix endianness issue which fixes deadlock#88
pranavkaruvally wants to merge 1 commit intogoogle:mainfrom
pranavkaruvally:s390x-fix

Conversation

@pranavkaruvally
Copy link

Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1121387

The threadpool having 8 threads, 1 thread was doing the actual work while the other 7 threads were waiting until the thread which was doing the work becomes available again (Waiting for the number of active threads to change). So, the following line that the main thread was executing contained the issue:

futex_wait((pthreadpool_atomic_uint32_t*)&threadpool->work_is_done, work_is_done);

threadpool->work_is_done is of type _Atomic unsigned long and inside the function futex_wait, the variable is directly passed to syscall(SYS_futex, address, val, ...) where the address was expected to have uint32_t*.
Although, the threadpool->work_is_done had a value of 1, reading the upper 32-bits made it wait (and work_is_done has value 0) and hence waits. Originally, this thread should not have blocked, should have proceeded further and should have called futex_wake_all to awaken the other 7 threads which were waiting for the completion of this thread's execution.
And, looking back at older commits, pthreadpool_atomic_uint32_t used to be _Atomic(uint32_t) or uint32_t depending on the compiler (conditional compilation). On a specific commit it was decided to use the stdatomic.h header file to replace this and used typedef atomic_uint_fast32_t threadpool_atomic_uint32_t instead.

And atomic_uint_fast32 means: an atomic, unsigned integer that is at least 32 bits and chosen for fast hardware access. Hence it is not necessarily 32-bit on every machine, on the contrary is 64-bit on most machines. Changing that to least32_t guarantees it to have 32-bits on all machines that support a 32-bit wide integer type (All linux machines I suppose).

@google-cla
Copy link

google-cla bot commented Jan 20, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@pranavkaruvally
Copy link
Author

The change should be to _Atomic uint32_t to be precise. Please mention that if we need to use that so that I can change my PR.
Thanks

@pranavkaruvally
Copy link
Author

Hi! I've signed the CLA now. Could someone please take a look at this when you have a chance? Thank you!

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.

1 participant