Skip to content

Commit

Permalink
LocalDispatcher : Use WeakMethod for BackgroundTask
Browse files Browse the repository at this point in the history
This was made necessary by changes in GafferHQ#5579
which meant that the functor wasn't destroyed immediately after running. Using the
WeakMethod is better though, because the old method wouldn't have broken the circular
reference if the background task was cancelled before it was started.
  • Loading branch information
johnhaddon committed Dec 12, 2023
1 parent eb97ba3 commit cb5699a
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
2 changes: 1 addition & 1 deletion python/GafferDispatch/LocalDispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def messagesChangedSignal( self ) :
def _execute( self ) :

if self.__executeInBackground :
self.__backgroundTask = Gaffer.BackgroundTask( None, self.__executeInternal )
self.__backgroundTask = Gaffer.BackgroundTask( None, Gaffer.WeakMethod( self.__executeInternal ) )
else :
self.__executeInternal()

Expand Down
13 changes: 12 additions & 1 deletion python/GafferDispatchTest/LocalDispatcherTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

import datetime
import errno
import gc
import os
import stat
import shutil
Expand All @@ -60,6 +61,16 @@

class LocalDispatcherTest( GafferTest.TestCase ) :

def tearDown( self ) :

GafferTest.TestCase.tearDown( self )

# Check for Jobs existing past their expected lifetime (most likely through circular references).
self.assertEqual(
[ o for o in gc.get_objects() if isinstance( o, GafferDispatch.LocalDispatcher.Job ) ],
[]
)

def __createLocalDispatcher( self, jobPool = None ) :

if jobPool is None :
Expand Down Expand Up @@ -528,7 +539,7 @@ def testFailure( self ) :
self.assertFalse( os.path.isfile( s.context().substitute( s["n2"]["fileName"].getValue() ) ) )
self.assertFalse( os.path.isfile( s.context().substitute( s["n1"]["fileName"].getValue() ) ) )

self.tearDown()
os.unlink( s.context().substitute( s["n3"]["fileName"].getValue() ) )

dispatcher["executeInBackground"].setValue( True )
dispatcher.dispatch( [ s["n1"] ] )
Expand Down

0 comments on commit cb5699a

Please sign in to comment.