larryhastings / gilectomy

Gilectomy branch of CPython. Use "gilectomy" branch in git. Read the important, short README below!
Other
527 stars 43 forks source link

threading locks seem to be thread-unsafe (sic!) #36

Open JustAMan opened 8 years ago

JustAMan commented 8 years ago

When running the following script

import threading                          

lock = threading.Lock()                   
def task():                               
    while True:                           
        with lock:                        
            pass                          

for _ in range(10):                       
    threading.Thread(target=task).start() 

I'm always sooner or later getting the following traceback:

Traceback (most recent call last):                                                         
  File "/..../gilectomy/Lib/threading.py", line 917, in _bootstrap_inner
    self.run()                                                                             
  File "/..../gilectomy/Lib/threading.py", line 865, in run             
    self._target(*self._args, **self._kwargs)                                              
  File "locksafety.py", line 7, in task                                                 
    pass                                                                                   
RuntimeError: release unlocked lock                                                        

I'm trying to figure out what's wrong but so far I have no clue... any suggestions welcome.

P.S. This is a reduced reproducer for the issue that was sometimes seen when launching x.py

JustAMan commented 8 years ago

I think I found the problem... Looking at _threading.c it seems that this is due to the fact that operations on lock object are not done atomically, instead they're done in two parts - first acquiring/releasing a lock, then writing down some status variable (self->locked, to be precise).

So I think the following scenario is possible:

  1. Two threads simultaneously enter lock_PyThread_acquire_lock function (or lock.__enter__() method, if you like)
  2. One of them succeeds on grabbing the lock and sets lock->locked to 1, another is blocked waiting
  3. Then first thread calls lock_PyThread_release_lock, which first unlocks underlying lock... and here's the data race - second thread wakes up and sets lock->locked to 1, but first thread after that, thinking it unlocked everything, sets lock->locked to 0

So this leads to lockobject structure being in inconsistent state - flag is saying "I'm unlocked" while underlying semaphore is taken.

Not sure how to fix this, though...

larryhastings commented 8 years ago

It's ironic that the _threading module isn't thread-safe ;-)

But it's not that shocking. I'll read it over myself and get back to you.

(p.s. good catch!)

JustAMan commented 8 years ago

Just in case - I left a PR ( #37 ) that reduces one data race possibility to almost zero (though it does not remove it completely). I think merging it won't hurt anyone :)

renejsum commented 7 years ago

Probably related:

Traceback (most recent call last):
  File "/projects/gilectomy/Lib/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/projects/pyworks/pyworks/taskmanager.py", line 150, in run
    m = self.queue.get(timeout=self.task._timeout)
  File "/projects/gilectomy/Lib/queue.py", line 176, in get
    return item
  File "/projects/gilectomy/Lib/threading.py", line 243, in __exit__
    return self._lock.__exit__(*args)
RuntimeError: release unlocked lock
JustAMan commented 7 years ago

Most certainly it is related, this was more or less the issue I was investigating when I found threading to be unsafe.

Thanks, Vasily 24 сент. 2016 г. 2:18 пользователь "Rene Nejsum" notifications@github.com написал:

Probably related:

Traceback (most recent call last): File "/projects/gilectomy/Lib/threading.py", line 916, in

_bootstrap_inner self.run() File "/projects/pyworks/pyworks/taskmanager.py", line 150, in run m = self.queue.get(timeout=self.task.timeout) File "/projects/gilectomy/Lib/queue.py", line 176, in get return item File "/projects/gilectomy/Lib/threading.py", line 243, in exit_ return self._lock.exit__(*args) RuntimeError: release unlocked lock

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/larryhastings/gilectomy/issues/36#issuecomment-249323917, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9vRi296Ep-upjk1F1_JZ-E6FEbAqT7ks5qtF4ugaJpZM4I3dm7 .