id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
21085	Race condition in BoundMethodWeakref used by signal mechanism.	Graham Dumpleton	nobody	"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
}}}

"	Bug	closed	Core (Other)	1.5	Normal	invalid			Accepted	0	0	0	0	0	0