classification
Title: class mutex doesn't do anything atomically
Type: Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, christian.heimes, dbenbenn, facundobatista, humitos, ncoghlan, orsenthil, pitrou
Priority: normal Keywords: easy, patch

Created on 2007-07-01 14:49 by dbenbenn, last changed 2008-02-23 22:54 by facundobatista. This issue is now closed.

Files
File name Uploaded Description Edit
ex.py dbenbenn, 2007-07-02 09:23 Example of two threads locking the same mutex.
patch.txt dbenbenn, 2007-07-03 21:26 Add locks to mutex.py
mutex.diff humitos, 2008-02-23 16:26
test_mutex.diff benjamin.peterson, 2008-02-23 22:05 TestCase for mutex
Messages (13)
msg32432 - (view) Author: David Benbennick (dbenbenn) Date: 2007-07-01 14:49
    >>> import mutex
    >>> print mutex.mutex.testandset.__doc__
    Atomic test-and-set -- grab the lock if it is not set,
            return True if it succeeded.


The above docstring is wrong: the method is not atomic.  This is easy to see by inspecting the method's code:

    def testandset(self):
        """Atomic test-and-set -- grab the lock if it is not set,
        return True if it succeeded."""
        if not self.locked:
            self.locked = 1
            return True
        else:
            return False

Therefore, it is possible for two threads to lock the same mutex simultaneously.  So the mutex module cannot be used for mutual exclusion.

The documentation for mutex says "The mutex module defines a class that allows mutual-exclusion via acquiring and releasing locks."  [http://docs.python.org/lib/module-mutex.html].  Perhaps it would be a good idea to make the module actually do what the documentation says.
msg32433 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2007-07-02 03:10
Hi David,
I just fired up the docs and found this:
"The mutex module defines a class that allows mutual-exclusion via acquiring and releasing locks. It does not require (or imply) threading or multi-tasking, though it could be useful for those purposes."

The docs dont say about threads using mutex object, but instead say if you want to use threading you can use mutex obj.

How are you using mutex with threads, can you please provide some information.

If muobj is an instance of mutex class.
muobj.testandset() for process-a will set the lock.
muobj.testandset() for process-b will be dealt with self.lock = True and wont be able to set.
msg32434 - (view) Author: David Benbennick (dbenbenn) Date: 2007-07-02 09:23
> How are you using mutex with threads, can you please provide some
information.

I'm attaching an example program that demonstrates two threads both locking the same mutex at the same time.

> If muobj is an instance of mutex class.
> muobj.testandset() for process-a will set the lock.
> muobj.testandset() for process-b will be dealt with self.lock = True and
> wont be able to set.

That isn't correct.  It is possible for testandset to return True in both thread-a and thread-b.  What can happen is the following:

1) Thread a calls testandset().  It executes the line "if not self.locked", and finds the result to be True.
2) The OS switches threads.
3) Thread b calls testandset().  It executes the line "if not self.locked", and finds the result to be True.
4) Thread b sets "self.locked = 1" and returns True
5) Thread a sets "self.locked = 1" and returns True
File Added: ex.py
msg32435 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2007-07-03 19:36
Thanks David, there is something 'interesting' being observed here.
At a point:
Calling testandset in thread 1, m.locked is False
Calling testandset in thread 0, m.locked is False
Thread 0 locked
Resetting, trying again

Another place:
Calling testandset in thread 1, m.locked is False
Calling testandset in thread 0, m.locked is False
Thread 0 locked
Thread 1 locked
Hah, all these threads locked at the same time: [0, 1]

My doubts are still with threading, but am unable to derive anything.
Should someone more experienced look into? Or mind taking this for suggestions to c.l.p?

msg32436 - (view) Author: David Benbennick (dbenbenn) Date: 2007-07-03 21:26
I've attached a patch to mutex.py that fixes the bug by acquiring a lock in testandset() and unlock().  After you apply the patch, the previous attachment will run forever.
File Added: patch.txt
msg32437 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2007-07-31 13:18
The docs may be misleading - they're missing a key comment from the source code that suggests that this particular mutex is specifically for use with the single threaded event scheduler in the sched module. On the other hand, the module predates the thread module by more than a year, and the threading module by about 7 years, so it may simply have failed to keep up with the times.

The module as written definitely isn't thread-safe though (I suspect the question of whether or not it should be hasn't really come up before, as most new code just uses threading.Lock or threading.RLock for all its mutex needs). Either the code needs to change or the docs - this question would probably be best asked on python-dev (referencing this bug report).
msg60167 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-01-19 14:21
Making the mutex module thread safe is a low hanging fruit for the bug
day. *wink*
msg60230 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-01-19 20:47
> Making the mutex module thread safe is a low hanging fruit for the bug
day.

But would it bring anything that the threading module doesn't have?
msg62766 - (view) Author: Manuel Kaufmann (humitos) * Date: 2008-02-23 16:26
I tested the patch and I found an error. It was missing a line "if 
call:" in the unlock function. I added it, and tested again with the 
ex.py example and it didn't found any threads locked at the same time.
msg62769 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-02-23 16:35
Is this module still of any use to anyone? It seems if you wanted a fake
mutex, you could just import LockType from dummy_thread.
msg62815 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-02-23 21:45
On 2008/2/23, Guido van Rossum said in python-dev

> According to the docstring it's only meant to be used with sched.py.
> Please don't try to make it work with threads!

Anyway, this module will be removed, or at least its API hidden, in 3.0.
msg62819 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-02-23 22:05
For what it's worth, here's a test case against the trunk.
msg62832 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-02-23 22:54
Added test case in r61024.
History
Date User Action Args
2008-02-23 22:54:31facundobatistasetmessages: + msg62832
2008-02-23 22:05:49benjamin.petersonsetfiles: + test_mutex.diff
keywords: + patch
messages: + msg62819
2008-02-23 21:45:23facundobatistasetstatus: open -> closed
resolution: wont fix
messages: + msg62815
nosy: + facundobatista
2008-02-23 16:35:42benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg62769
2008-02-23 16:26:11humitossetfiles: + mutex.diff
nosy: + humitos
messages: + msg62766
2008-01-19 20:47:28pitrousetnosy: + pitrou
messages: + msg60230
2008-01-19 14:21:46christian.heimessetkeywords: + easy
nosy: + christian.heimes
messages: + msg60167
versions: + Python 2.6, - Python 2.5
2007-07-01 14:49:39dbenbenncreate