LibC: Unblock all threads in pthread_cond_broadcast

[mich181189]

pthread_cond_broadcast is like pthread_cond_signal except it wakes ALL threads waiting on the condition variable.

As detailed in #15020, it appears to only be waking a single thread. Further investigation of the implementation suggests that it was in fact set to only wake one thread.

This commit changes the futex call in pthread_cond_broadcast to wake at most INT_MAX tasks, rather than at most 1 task. It also sets the val3 argument to 0 rather than INT_MAX since it is unused, and 0 feels like a better "unused" value.

[timschumi]

Are you changing the correct argument here? For FUTEX_CMP_REQUEUE (and, with that, FUTEX_REQUEUE), the futex manpage says:

long syscall(SYS_futex, uint32_t *uaddr, int futex_op, uint32_t val,
                    const struct timespec *timeout,   /* or: uint32_t val2 */
                    uint32_t *uaddr2, uint32_t val3);

[...] Otherwise, the operation wakes up a maximum of val waiters that are waiting on the futex at uaddr. If there are more than val waiters, then the remaining waiters are removed from the wait queue of the source futex at uaddr and added to the wait queue of the target futex at uaddr2. The val2 argument specifies an upper limit on the number of waiters that are requeued to the futex at uaddr2. [...] Typical values to specify for val are 0 or 1. (Specifying INT_MAX is not useful, because it would make the FUTEX_CMP_REQUEUE operation equivalent to FUTEX_WAKE.) The limit value specified via val2 is typically either 1 or INT_MAX. (Specifying the argument as 0 is not useful, because it would make the FUTEX_CMP_REQUEUE operation equivalent to FUTEX_WAIT.)

Does this mean that our futex implementation is nonstandard, or is our pthread_cond_broadcast still not compliant after this but barely manages to pass through?

[mich181189]

Hmm, yeah you might be right @timschumi I didn't see that bit.

I don't see a way to make it work any other way though - the futex syscall (and maybe some of the userspace) seems to assume that value is a timeout rather than val2, so various experiments have resulted in crashes... so maybe that needs fixing as well

In summary, it might be both that this is suboptimal (since it may as well use FUTEX_WAKE as the docs say) and also that the syscall doesn't really support it either

[mich181189]

That's updated now to first fix futex() then use it as I understand the documentation intends

[bgianfo]

Since the bug has an existing test, would you mind adding that as well under Tests/LibC? There are some existing pthread tests there that could be used as a guide.

[BuggieBot]

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

[stale[bot]]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!