libc/pthread: return EBUSY when barrier is in use

[xiaoxiang781216]

Summary

specified here: https://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_barrier_destroy.html

Impact

More confirm to the standard.

Testing

[gustavonihei]

Please, squash these two commits: https://github.com/apache/incubator-nuttx/pull/4129/commits/ea0ddd188e222dadcaa6e0860ad97ebf79656969 / https://github.com/apache/incubator-nuttx/pull/4129/commits/790639c769bf4fea1677e7bb16fb069b7a77fa22

[xiaoxiang781216]

Done.

[yamt]

do you have any real code relying on pthread_barrier_destroy detecting it?

[xiaoxiang781216]

when pthread_barrier_init is called, the barrier is an uninitialized garbage. it's value should not be checked.

LTP testcase call pthread_barrier_init twice, and expect the second call return fail: https://github.com/linux-test-project/ltp/blob/master/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrier_init/4-1.c#L81-L85 https://github.com/linux-test-project/ltp/blob/master/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrier_init/4-1.c#L113-L123 Do you have better method to fix it?

do you have any real code relying on pthread_barrier_destroy detecting it?

No, but it specify in the standard and check by LTP: https://github.com/linux-test-project/ltp/blob/master/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrier_destroy/2-1.c#L121-L131 We must to follow.

[Ouss4]

@xiaoxiang781216 please see the latest issue: https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_barrier_destroy.html

It says (at the bottom):

The [EBUSY] error for a barrier that is in use or an already initialized barrier object is removed; this condition results in undefined behavior.

EDIT: It also says:

If an implementation detects that the value specified by the barrier argument to pthread_barrier_destroy() or pthread_barrier_init() refers to a barrier that is in use (for example, in a pthread_barrier_wait() call) by another thread, or detects that the value specified by the barrier argument to pthread_barrier_init() refers to an already initialized barrier object, it is recommended that the function should fail and report an [EBUSY] error.

[xiaoxiang781216]

Ok, the change just check destroy path now, please take a look @yamt

[yamt]

my reading of the standard is that if an implementation detects the condition, it can report EBUSY. it seems fine not to detect the condition. IMO this is something which shouldn't be "fixed". ltp is just broken or probably checking too specific implementation behavior.

[yamt]

i glanced at the latest nptl code of pthread_barrier_destroy. it doesn't seem to return EBUSY either. (older versions seem to do)

[xiaoxiang781216]

my reading of the standard is that if an implementation detects the condition, it can report EBUSY. it seems fine not to detect the condition. IMO this is something which shouldn't be "fixed". ltp is just broken or probably checking too specific implementation behavior.

It isn't an implementation behaviour, it specify and recommand in standard. https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_barrier_destroy.html:

If an implementation detects that the value specified by the barrier argument to pthread_barrier_destroy() or pthread_barrier_init() refers to a barrier that is in use (for example, in a pthread_barrier_wait() call) by another thread, or detects that the value specified by the barrier argument to pthread_barrier_init() refers to an already initialized barrier object, it is recommended that the function should fail and report an [EBUSY] error.

i think it's better not to "fix" this.

And openbsd also return EBUSY too: https://github.com/openbsd/src/blob/2207c4325726fdc5c4bcd0011af0fdf7d3dab137/lib/librthread/rthread_barrier.c#L86-L89

[xiaoxiang781216]

i think it's better not to "fix" this.

@yamt please give a reasonable explain why it's better to not fix it, even the standard explicitly state that "it is recommended that the function should fail and report an [EBUSY] error."

[yamt]

i think it's better not to "fix" this.

@yamt please give a reasonable explain why it's better to not fix it, even the standard explicitly state that "it is recommended that the function should fail and report an [EBUSY] error."

the EBUSY here is optional. (that's why the standard removed it from its ERRORS section.) a portable application can not rely on it anyway. less code is better for us.

[yamt]

a portable application can not rely on it anyway.

i guess i will change my mind if you give me an example of real applications relying on this EBUSY detection. (real == not a test code)