Race condition in BoundMethodWeakref used by signal mechanism.
|Reported by:||grahamd||Owned by:||nobody|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Description (last modified by akaariai)
In the __new__ method of BoundMethodWeakref at:
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
Change History (4)
comment:1 Changed 6 months ago by akaariai
- Component changed from Uncategorized to Core (Other)
- Description modified (diff)
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Accepted
- Type changed from Uncategorized to Bug