[New-bugs-announce] [issue11618] Locks broken wrt timeouts on Windows

[sbt]

New submission from sbt <shibturn at gmail.com>:

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.

----------
files: test-timeout.py
messages: 131515
nosy: sbt
priority: normal
severity: normal
status: open
title: Locks broken wrt timeouts on Windows
type: behavior
versions: Python 3.2, Python 3.3
Added file: http://bugs.python.org/file21304/test-timeout.py

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue11618>
_______________________________________