Dispatcher : Call SetupPlugsFn _after_ construction, not during

This uses an `intrusive_ptr_add_ref` gambit first introduced in GafferHQ#5985, with the rationale for that over various alternatives being documented there.

Fixes GafferHQ#915.

Changes.md

@@ -27,6 +27,7 @@ Fixes
 - Shuffle, ShuffleAttributes, ShufflePrimitiveVariables : Fixed some special cases where shuffling a source to itself would fail to have the expected effect.
 - GraphEditor : Fixed dimming of labels for BoxIn and BoxOut nodes.
 - GafferCortexUI : Removed usage of legacy PlugValueWidget API.
+- Dispatcher : Fixed crashes caused by a dispatcher's `SetupPlugsFn` attempting to access the TaskNode it was being called for. Dispatchers may now introspect the TaskNode and add different plugs based on type (#915).
 
 API
 ---

include/GafferDispatch/Dispatcher.h

@@ -201,8 +201,6 @@ class GAFFERDISPATCH_API Dispatcher : public TaskNode
 
 	protected :
 
-		friend class TaskNode;
-
 		IE_CORE_FORWARDDECLARE( TaskBatch )
 
 		using TaskBatches = std::vector<TaskBatchPtr>;
@@ -270,16 +268,12 @@ class GAFFERDISPATCH_API Dispatcher : public TaskNode
 		/// batches have been dispatched in order to prevent duplicate work.
 		virtual void doDispatch( const TaskBatch *batch ) const = 0;
 
-		//! @name TaskNode Customization
-		/// Dispatchers are able to create custom plugs on TaskNodes when they are constructed.
-		/////////////////////////////////////////////////////////////////////////////////////////////
-		//@{
-		/// Adds the custom plugs from all registered Dispatchers to the given parent Plug.
-		static void setupPlugs( Gaffer::Plug *parentPlug );
-		//@}
-
 	private :
 
+		// Friendship to allow `setupPlugs()` to be called.
+		friend void intrusive_ptr_add_ref( TaskNode *node );
+		static void setupPlugs( Gaffer::Plug *parentPlug );
+
 		void preTasks( const Gaffer::Context *context, Tasks &tasks ) const final;
 		void postTasks( const Gaffer::Context *context, Tasks &tasks ) const final;
 		IECore::MurmurHash hash( const Gaffer::Context *context ) const final;

include/GafferDispatch/TaskNode.h

@@ -235,4 +235,6 @@ class GAFFERDISPATCH_API TaskNode : public Gaffer::DependencyNode
 
 };
 
+GAFFERDISPATCH_API void intrusive_ptr_add_ref( TaskNode *node );
+
 } // namespace GafferDispatch

python/GafferDispatchTest/DispatcherTest.py

@@ -2313,5 +2313,29 @@ def testDispatchSignalShutdownCrash( self ) :
 			"""import GafferDispatch; GafferDispatch.Dispatcher.postDispatchSignal().connect( lambda d, s : None, scoped = False )"""
 		] )
 
+	def testAccessTaskNodeInSetupPlugs( self ) :
+
+		class SetupPlugsTestDispatcher( GafferDispatch.Dispatcher ) :
+
+			def _doDispatch( self, batch ) :
+
+				pass
+
+			lastNode = None
+
+			@classmethod
+			def _setupPlugs( cls, parentPlug ) :
+
+				node = parentPlug.node()
+				self.assertIsInstance( node, GafferDispatch.PythonCommand )
+				self.assertEqual( node.typeId(), GafferDispatch.PythonCommand.staticTypeId() )
+				cls.lastNode = node
+
+		GafferDispatch.Dispatcher.registerDispatcher( "SetupPlugsTestDispatcher", SetupPlugsTestDispatcher, SetupPlugsTestDispatcher._setupPlugs )
+		self.addCleanup( GafferDispatch.Dispatcher.deregisterDispatcher, "SetupPlugsTestDispatcher" )
+
+		pythonCommand = GafferDispatch.PythonCommand()
+		self.assertIs( SetupPlugsTestDispatcher.lastNode, pythonCommand )
+
 if __name__ == "__main__":
 	unittest.main()

src/GafferDispatch/TaskNode.cpp

@@ -286,8 +286,6 @@ TaskNode::TaskNode( const std::string &name )
 
 	PlugPtr dispatcherPlug = new Plug( "dispatcher", Plug::In );
 	addChild( dispatcherPlug );
-
-	Dispatcher::setupPlugs( dispatcherPlug.get() );
 }
 
 TaskNode::~TaskNode()
@@ -400,3 +398,17 @@ bool TaskNode::requiresSequenceExecution() const
 {
 	return false;
 }
+
+void GafferDispatch::intrusive_ptr_add_ref( TaskNode *node )
+{
+	bool firstRef = node->refCount() == 0;
+	node->addRef();
+	if( firstRef )
+	{
+		// We defer calling `setupPlugs()` till we have our first reference, as
+		// otherwise Python `setupPlugs()` functions which accessed
+		// `plug.node()` would crash. This also means that `setupPlugs()` is able
+		// to introspect the node type and add different plugs on different nodes.
+		Dispatcher::setupPlugs( node->dispatcherPlug() );
+	}
+}