[libdispatch-dev] [PATCH 09/17] portability

[Kevin Van Vechten]

On Oct 28, 2009, at 11:03 AM, Joakim Johansson wrote:

> I agree, it would be a good change for robustness, on Solaris 10 it is not a scalar even - I had the following code in my local porting tree while still figuring things out for later revisiting:
> 
> -------------
> 
> static void
> _dispatch_posix_semaphore_create(sem_t *s4)
> {
> 	int ret;
> 
> /*
>   Commented this out for Solaris port, as sem_t is a structure on solaris and not a scalar.
>   This check seems unclear semantically, as it assumes that sem_t is of a certain type.
> 
>   if (*s4) {
> 		return;
> 	}
> */
> 	ret = sem_init(s4, 0, 0);
> 	dispatch_assume_zero(ret);
> }

While reviewing the semaphore implementation I noticed that _dispatch_posix_semaphore_create is not mirroring _dispatch_sempahore_create_port's semantics.

A dispatch semaphore starts life as an integer value that is incremented or decremented.  The more heavy-weight OS semaphore is allocated lazily only if a thread needs to block (or another thread believes that a thread is blocked on the semaphore and needs to be signaled).  The _dispatch_semaphore_create_port function allocates a new OS semaphore and then performs a dispatch_atomic_cmpxchg to initialize the field in the dispatch_semaphore_t.  If it loses the race (because another thread had already allocated an OS semaphore for the same dispatch_semaphore_t) then the create function destroys its speculative OS semaphore before returning and leaves the dispatch_semaphore_t unchanged.

// abridged _dispatch_semaphore_create_port and _dispatch_posix_semaphore_create

> #if HAVE_MACH
> static void
> _dispatch_semaphore_create_port(semaphore_t *s4) {
> 	if (*s4) return;
> 
> 	tmp = semaphore_create();
> 	if (!dispatch_atomic_cmpxchg(s4, 0, tmp)) {
> 		semaphore_destroy(tmp);
> 	}
> }
> #else /* !HAVE_MACH */
> static void
> _dispatch_posix_semaphore_create(sem_t *s4)
> {
> 	if (*s4) return;
> 	ret = sem_init(s4, 0, 0); // XXX race condition for assignment to s4
> }
> #endif /* HAVE_MACH */

Based on the above discussion it seems like we may need to make dispatch_semaphore_t be a thinner wrapper around OS semaphores on platforms where the semaphore's handle cannot be atomically exchanged — i.e. we may need to forgo the optimization of lazily allocating the OS semaphore.

Kevin