From 0a971d63073ca11a1af021d59568c0872225c62d Mon Sep 17 00:00:00 2001 From: John Haddon Date: Thu, 23 Nov 2023 13:16:56 +0000 Subject: [PATCH] LocalDispatcher : Use WeakMethod with `callOnUIThread()` This is probably neither here nor there in actual usage in the UI, but it is important for running the unit tests when the UI module has been imported. That's because `GafferUI.EventLoop` unconditionally installs a UIThreadCallHandler that just buffers the calls until an event loop is started. That prolonged Job lifetimes way outside of their associated tests, eventually causing this spooky action at a distance : ``` Traceback (most recent call last): File "/__w/gaffer/gaffer/build/python/GafferTest/BoxTest.py", line 1138, in testComputeNodeCastDoesntRequirePython node = CastChecker() File "/__w/gaffer/gaffer/build/python/GafferTest/BoxTest.py", line 1129, in __init__ self["out"] = Gaffer.IntPlug( direction = Gaffer.Plug.Direction.Out ) IECore.Exception: Traceback (most recent call last): File "/__w/gaffer/gaffer/build/python/GafferTest/BoxTest.py", line 1133, in isInstanceOf raise Exception( "Cast to ComputeNode should not require Python" ) Exception: Cast to ComputeNode should not require Python ``` How did that happen? Well, a Job contains a BackgroundTask, and `self["out"]` is a graph edit, and graph edits cancel background tasks, and the cancellation code does a `runTimeCast()` on the subject of the edit. So if a BackgroundTask exists thanks to EventLoop, then the cancellation code runs and does a `runTimeCast()` on `CastChecker`, which throws. It really would be better if EventLoop scoped its UIThreadCallHandler between `start()` and `stop()`. --- python/GafferDispatch/LocalDispatcher.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/python/GafferDispatch/LocalDispatcher.py b/python/GafferDispatch/LocalDispatcher.py index daa6fb1988c..c963311455a 100644 --- a/python/GafferDispatch/LocalDispatcher.py +++ b/python/GafferDispatch/LocalDispatcher.py @@ -367,9 +367,9 @@ def __updateStatus( self, status ) : self.__status = status if threading.current_thread() is threading.main_thread() : - self.statusChangedSignal()( self ) + self.__emitStatusChanged() elif Gaffer.ParallelAlgo.canCallOnUIThread() : - Gaffer.ParallelAlgo.callOnUIThread( functools.partial( self.statusChangedSignal(), self ) ) + Gaffer.ParallelAlgo.callOnUIThread( Gaffer.WeakMethod( self.__emitStatusChanged, fallbackResult = None ) ) IECore.msg( IECore.MessageHandler.Level.Info, f"{self.__name} {self.__id}", str( status ) @@ -379,6 +379,10 @@ def __messagesChanged( self ) : self.__messagesChangedSignal( self ) + def __emitStatusChanged( self ) : + + self.statusChangedSignal()( self ) + class JobPool : def __init__( self ) : @@ -492,7 +496,7 @@ def handle( self, level, context, msg ) : return if Gaffer.ParallelAlgo.canCallOnUIThread() : - Gaffer.ParallelAlgo.callOnUIThread( self.__messagesChangedUICall ) + Gaffer.ParallelAlgo.callOnUIThread( Gaffer.WeakMethod( self.__messagesChangedUICall, fallbackResult = None ) ) def __messagesChangedUICall( self ) :