[Django] #21085: Race condition in BoundMethodWeakref used by signal mechanism.
Django
-------------------------------+--------------------
Reporter: grahamd | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
In the _new__ method of BoundMethodWeakref at:
https://github.com/django/django/blob/master/django/dispatch/saferef.py#L73
there is a potential threading race condition.
The code is:
{{{
def __new__( cls, target, onDelete=None, *arguments,**named ):
"""Create new instance or return current instance
Basically this method of construction allows us to
short-circuit creation of references to already-
referenced instance methods. The key corresponding
to the target is calculated, and if there is already
an existing reference, that is returned, with its
deletionMethods attribute updated. Otherwise the
new instance is created and registered in the table
of already-referenced methods.
"""
key = cls.calculateKey(target)
current =cls._allInstances.get(key)
if current is not None:
current.deletionMethods.append( onDelete)
return current
else:
base = super( BoundMethodWeakref, cls).__new__( cls )
cls._allInstances[key] = base
base.__init__( target, onDelete, *arguments,**named)
return base
}}}
The problem is that multiple threads could at the same time invoke
cls._allInstances.get(key) and get None.
They will both then create instances of BoundMethodWeakref but only one
will win as far as updating cls._allInstances.
Because all the onDelete callbacks are stored in the BoundMethodWeakref
instance which is added to the cache. the thread who lost as far as
getting their instance in the cache, will have its onDelete lost.
Code should in the else clause instead do something like:
{{{
base = super( BoundMethodWeakref, cls).__new__( cls )
base.__init__( target, onDelete, *arguments,**named)
instance = cls._allInstances.setdefault(key, base)
if instance is not base:
instance.deletionMethods.append(onDelete)
return instance
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21085>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: grahamd | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akaariai):
* needs_better_patch: => 0
* component: Uncategorized => Core (Other)
* needs_tests: => 0
* needs_docs: => 0
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Old description:
New description:
In the `__new__` method of BoundMethodWeakref at:
https://github.com/django/django/blob/master/django/dispatch/saferef.py#L73
The code is:
instance = cls._allInstances.setdefault(key, base)
return instance
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/21085#comment:1>
Django
Reporter: grahamd | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
See also #19511.
--
Ticket URL: <https://code.djangoproject.com/ticket/21085#comment:2>
Django
Reporter: grahamd | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vajrasky):
Is this bug related with this issue? With python 3.4:
{{{
$ python runtests.py null_fk
Creating test database for alias 'default'...
Creating test database for alias 'other'...
..
----------------------------------------------------------------------
Ran 2 tests in 0.063s
OK
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
Exception ignored in: <bound method Signal._remove_receiver of
<django.dispatch.dispatcher.Signal object at 0x7f662b75b330>>
Traceback (most recent call last):
File "/home/sky/Code/python/django/django/dispatch/dispatcher.py", line
276, in _remove_receiver
TypeError: 'NoneType' object is not callable
Exception ignored in: <bound method Signal._remove_receiver of
<django.dispatch.dispatcher.Signal object at 0x7f662b1be6d8>>
Traceback (most recent call last):
File "/home/sky/Code/python/django/django/dispatch/dispatcher.py", line
276, in _remove_receiver
TypeError: 'NoneType' object is not callable
Exception ignored in: <bound method Signal._remove_receiver of
<django.dispatch.dispatcher.Signal object at 0x7f662b1be6d8>>
Traceback (most recent call last):
...omitted...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21085#comment:3>
Django
Reporter: grahamd | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 1.5
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* status: new => closed
* resolution: => invalid
Comment:
Our implementation of weakrefs has now gone in favour of (backported) code
from python 3.4. This should hopefully no longer be an issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/21085#comment:4>