fixup! Context : Fix heap read after free

include/Gaffer/Context.h

@@ -357,11 +357,11 @@ class GAFFER_API Context : public IECore::RefCounted
 		};
 
 		// Sets a variable and emits `changedSignal()` as appropriate. Does not
-		// manage ownership in any way. If ownership is required, the caller must
-		// update `m_allocMap` appropriately _before_ calling `internalSet()`, _and_
-		// must keep any previous allocation alive until `internalSet()` has
-		// returned.
+		// manage ownership in any way. If ownership is required, call
+		// `internalSetWithOwner()` instead.
 		void internalSet( const IECore::InternedString &name, const Value &value );
+		// Sets a variable and maintains ownership of its data via `owner`.
+		void internalSetWithOwner( const IECore::InternedString &name, const Value &value, IECore::ConstDataPtr &&owner );
 		// Throws if variable doesn't exist.
 		const Value &internalGet( const IECore::InternedString &name ) const;
 		// Returns nullptr if variable doesn't exist.

include/Gaffer/Context.inl

@@ -155,16 +155,10 @@ void Context::Value::registerType()
 template<typename T, typename Enabler>
 void Context::set( const IECore::InternedString &name, const T &value )
 {
-	IECore::ConstDataPtr &ownedValue = m_allocMap[name];
-	// Keep old value alive for comparison with new value in `internalSet()`.
-	IECore::ConstDataPtr oldValue( std::move( ownedValue ) );
-	// Allocate new Data to own a copy of the value.
 	using DataType = typename Gaffer::Detail::DataTraits<T>::DataType;
 	typename DataType::ConstPtr d = new DataType( value );
-	const T *v = &d->readable();
-	ownedValue = std::move( d );
-	// Update `m_map`.
-	internalSet( name, Value( name, v ) );
+	const Value v( name, &d->readable() );
+	internalSetWithOwner( name, v, std::move( d ) );
 
 }
 
@@ -196,6 +190,18 @@ inline void Context::internalSet( const IECore::InternedString &name, const Valu
 	}
 }
 
+inline void Context::internalSetWithOwner( const IECore::InternedString &name, const Value &value, IECore::ConstDataPtr &&owner )
+{
+	IECore::ConstDataPtr &currentOwner = m_allocMap[name];
+	// Keep old value alive for comparison with new value in `internalSet()`.
+	IECore::ConstDataPtr oldOwner( std::move( currentOwner ) );
+	// Assign new owner so that we have a consistent internal state when
+	// `internalSet()` emits `changedSignal()`.
+	currentOwner = owner;
+	// Update `m_map`.
+	internalSet( name, value );
+}
+
 inline const Context::Value &Context::internalGet( const IECore::InternedString &name ) const
 {
 	const Value *result = internalGetIfExists( name );

src/Gaffer/Context.cpp

@@ -270,15 +270,15 @@ Context::Context( const Context &other, CopyMode mode )
 			)
 			{
 				// The value is already owned by `other`, and is immutable, so we
-				// can just add our own reference to it to share ownership
-				// and then call `internalSet()`.
-				m_allocMap[i.first] = allocIt->second;
-				internalSet( i.first, i.second );
+				// can just share ownership with it.
+				internalSetWithOwner( i.first, i.second, ConstDataPtr( allocIt->second ) );
 			}
 			else
 			{
 				// Data not owned by `other`. Take a copy that we own, and call `internalSet()`.
-				internalSet( i.first, i.second.copy( m_allocMap[i.first] ) );
+				ConstDataPtr owner;
+				const Value v = i.second.copy( owner );
+				internalSetWithOwner( i.first, v, std::move( owner ) );
 			}
 		}
 	}
@@ -310,14 +310,10 @@ Context::~Context()
 
 void Context::set( const IECore::InternedString &name, const IECore::Data *value )
 {
-	IECore::ConstDataPtr &ownedValue = m_allocMap[name];
-	// Keep old value alive for comparison with new value in `internalSet()`.
-	IECore::ConstDataPtr oldValue( std::move( ownedValue ) );
-	// Take ownership of a copy of the value so that the client can't invalidate
-	// this context by changing it.
-	ownedValue = value->copy();
-	// Update `m_map`.
-	internalSet( name, Value( name, ownedValue.get() ) );
+	// We copy the value so that the client can't invalidate this context by changing it.
+	ConstDataPtr copy = value->copy();
+	const Value v( name, copy.get() );
+	internalSetWithOwner( name, v, std::move( copy ) );
 }
 
 IECore::DataPtr Context::getAsData( const IECore::InternedString &name ) const