[PATCH] Fix return value and errno set by sem_init(), sem_destroy() and sem_close()

Jon TURNEY jon.turney@dronecode.org.uk
Mon Mar 28 22:13:00 GMT 2011


While looking into some mysterious failures of sem_init() in python, I was
somewhat surprised to find the following comment in python/thread_pthread.h:

> /*
>  * As of February 2002, Cygwin thread implementations mistakenly report error
>  * codes in the return value of the sem_ calls (like the pthread_ functions).
>  * Correct implementations return -1 and put the code in errno. This supports
>  * either.
>  */

While this comment refers to sem_wait() and sem_trywait(), which seem to have
been fixed since [1], it seems that sem_init(), sem_destroy() and sem_close()
are still non-conformant with SUS in that (i) they do not set errno, and (ii)
they don't return -1 on failure, instead returning the value which should be
set as errno.

I'm not sure if (ii) makes much practical difference, as portable code should
arguably be comparing the result against 0, although this is not helped by the
fact that (as the linux man page puts it) "Bizarrely, POSIX.1-2001 does not
specify the value that should be returned by a successful call to sem_init().
 POSIX.1-2008 rectifies this, specifying the zero return on success."

(i) causes the reason for a sem_init() failure to be incorrectly reported as
whatever the previous value of errno is, more on which anon.

Attached is a patch which fixes this conformance issue with these functions.

I did wonder if there were internal uses in the cygwin DLL of these functions
which might be affected by this change. In a quick audit, the only point of
concern I found was semaphore::_terminate(). semaphore::_terminate() is only
used by semaphore::terminate() and does not propagate the result, but now may
be setting errno.  I guess that semaphore::terminate() will now normally be
setting errno, as it attempts to apply semaphore::_terminate() to all
semaphores, not just shared ones.

semaphore::terminate() is only called from close_all_files().  Having this
change errno when used from do_exit() seems safe as we are past the point
where errno might be of significance, but the other uses of close_all_files()
are a bit more mysterious to me.

In what is probably an excess of caution, I've added a save_errno to
semaphore::terminate(), but I could use some help in determining if that is
actually needed.

2011-03-28  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* thread.cc (semaphore::init, destroy, close): Standards conformance
	fix.  On a failure, return -1 and set errno.
	* thread.h (semaphore::terminate): Save errno since semaphore::close()
	may now modify it.

[1] http://cygwin.com/ml/cygwin/2002-02/msg01379.html
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sem_init_errno_fix.patch
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20110328/9f765384/attachment.ksh>


More information about the Cygwin-patches mailing list