Author sbt
Recipients sbt
Date 2011-03-20.17:06:56
SpamBayes Score 3.33067e-16
Marked as misclassified No
Message-id <1300640817.81.0.852617582602.issue11618@psf.upfronthosting.co.za>
In-reply-to
Content
In thread_nt.h, when the WaitForSingleObject() call in
EnterNonRecursiveMutex() fails with WAIT_TIMEOUT (or WAIT_FAILED) the
mutex is left in an inconsistent state.

Note that the first line of EnterNonRecursiveMutex() is the comment

    /* Assume that the thread waits successfully */

Allowing EnterNonRecursiveMutex() to fail with a timeout obviously
violates this promise ;-)  I think the problem was introduced to Python
3.2 with:

     Issue7316: Add a timeout functionality to common locking operations.


The following Windows session demonstrates unexpected behaviour:

Python 3.3a0 (default, Mar 19 2011, 18:16:48) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import threading
>>> l = threading.Lock()
>>> l.acquire()
True
>>> l.acquire(timeout=1)
False
>>> l.release()
>>> l.locked()                  # should return False
True
>>> l.acquire(blocking=False)   # should return True
False


Also, after a timeout, uncontended acquires/releases always take the
slow path:

D:\Repos\cpython\PCbuild>python -m timeit ^
More? -s "from threading import Lock; l = Lock()" ^
More? "l.acquire();l.release()"
1000000 loops, best of 3: 0.974 usec per loop

D:\Repos\cpython\PCbuild>python -m timeit ^
More? -s "from threading import Lock; l = Lock()" ^
More? -s "l.acquire();l.acquire(timeout=0.1);l.release()" ^
More? "l.acquire();l.release()"
100000 loops, best of 3: 2.18 usec per loop


A unit test is attached which passes on Linux but has three failures
on Windows.


The "owned" field of NRMUTEX is a count of the number of threads
waiting for the mutex (not including the owner).  "owned" will
over-estimate the number of waiters if a timeout occurs, because the
timed out thread will still be counted as a waiter.

The obvious fix is to decrement mutex->owned when a timeout occurs.
Unfortunately that would introduce a race which might allow two
threads to think they own the lock at the same time.

I also notice that EnterNonRecursiveMutex() wrongly sets
mutex->thread_id to the current thread even when it fails with a
timeout.  It appears that the thread_id field is never actually used
-- is it there to help with debugging?  Perhaps it should just be
removed.

BTW only thread_pthread.h and thread_nt.h have implementations of
PyThread_acquire_lock_timed().  Since this function appears to be
required by _threadmodule.c, does this mean that in Python 3.2
threads are only supported with pthreads and win32?  If so you can
get rid of all those other thread_*.h files.
History
Date User Action Args
2011-03-20 17:06:57sbtsetrecipients: + sbt
2011-03-20 17:06:57sbtsetmessageid: <1300640817.81.0.852617582602.issue11618@psf.upfronthosting.co.za>
2011-03-20 17:06:57sbtlinkissue11618 messages
2011-03-20 17:06:56sbtcreate