Skip to content

Commit

Permalink
LocalDispatcher : Use WeakMethod with callOnUIThread()
Browse files Browse the repository at this point in the history
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()`.
  • Loading branch information
johnhaddon committed Nov 23, 2023
1 parent e96651a commit 0a971d6
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions python/GafferDispatch/LocalDispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand All @@ -379,6 +379,10 @@ def __messagesChanged( self ) :

self.__messagesChangedSignal( self )

def __emitStatusChanged( self ) :

self.statusChangedSignal()( self )

class JobPool :

def __init__( self ) :
Expand Down Expand Up @@ -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 ) :

Expand Down

0 comments on commit 0a971d6

Please sign in to comment.