PTHREAD_RWLOCK_INITIALIZER macro not defined

[cian]

The pthread_rwlock functions exist in the header and I presume work, but the pthread_rwlock_initializer macro does not exist.

I've had to take a guess at the component, its likely not correct.

[luroh]

Move POSIX compatibility related tickets out of R1 milestone (FutureHaiku/Features).

[korli]

FYI this is required by Blender.

[diver]

Also for current VLC.

[return_0e]

Patch to define PTHREAD_RWLOCK_INITIALIZER

[return_0e]

This macro is defined here ​http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_destroy.html and is also required for compiling with pthread support for Swift.

[waddlesplash]

Applied in hrev51184. Thanks!

[korli]

This patch assumes the implementation knows what to do with the values provided by pthread_rwlock_initializer, which it doesn't seem to. Best would be to provide a unit test for this, eventually.

[korli]

Please revert hrev51184.

[pulkomandy]

Reverted in hrev51190. See haiku-commits discussions on hrev51184 to see how this does not seem possible with the current implementation.

[korli]

Try at implementing local pthread_rwlock

[korli]

Tested on x86_64 only. I tried to implement again the local pthread_rwlock with mutexes. Might not be the best solution, the old one used block_thread and unblock_thread syscalls. Please review.

Another problem is the size of the struct _pthread_rwlock, it stays the same on x86_64 (40 bytes), not on x86 (was 32 bytes). I suppose we can reduce to only use 32 bytes on x86 and let 8 bytes unused on x86_64. Can this size actually change? Or are we stucked with these sizes for compatibility reasons?

[axeld]

Since Haiku introduced the pthread_* functions in the first place, and we didn't reach beta yet, I think it's fair to say that it's okay to break compatibility at this point. It would also be nice to be able to actually break old code visibly, not just let it crash in mysterious ways... Maybe we can achieve this with symbol versioning?

I would probably add a few bytes for future changes, though :-)

[korli]

reworked patch for local pthread_rwlock

[korli]

@axeld I've changed the implementation to use only the first 32 bytes of the struct, so we could theoretically add a few bytes at the end: this should normally have no impact on existing binaries (TODO: remove the #ifdef B_HAIKU_64_BIT).

[bonefish]

Unfortunately I don't have the time to thoroughly read the patch. From a quick glance: Not using a common waiting queue for readers and writers means that you can't treat both fairly. In this case it looks like writers can overtake waiting readers. As I wrote on the commit mailing list, I'd just replace the semaphore by a mutex and leave the implementation otherwise unchanged.

[korli]

bonefish, yes readers and writers are treated unfairly in the patch, as in the specification: "The calling thread acquires the read lock if a writer does not hold the lock and there are no writers blocked on the lock.", which actually means that writers can overtake waiting readers. ​http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_rdlock.html

[bonefish]

Replying to korli:

bonefish, yes readers and writers are treated unfairly in the patch, as in the specification: "The calling thread acquires the read lock if a writer does not hold the lock and there are no writers blocked on the lock.", which actually means that writers can overtake waiting readers. ​http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_rdlock.html

But that only means that a reader may not overtake an already waiting writer. In your implementation a new writer also overtakes an already waiting reader. While the specification doesn't seem to disallow that behavior, it means readers can be starved (which cannot happen with the current common queue implementation).

[korli]

try 3

[korli]

Applied in hrev51310.

[korli]

Not seen a regression so far. Closing.

[bitigchi]

Is this still valid? It looks like latest VLC still needed a ​workaround.

[waddlesplash]

No, it is definitely defined: ​https://github.com/haiku/haiku/blob/master/headers/posix/pthread.h#L83

The VLC patch may just be old and can be deleted.