Skip to content

Commit

Permalink
Catalogue : Fix bug when orphaned Catalogue tries to save an image.
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewkaufman committed Jun 20, 2018
1 parent da35c31 commit 77cb07d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
43 changes: 40 additions & 3 deletions python/GafferImageTest/CatalogueTest.py
Expand Up @@ -49,19 +49,19 @@
class CatalogueTest( GafferImageTest.ImageTestCase ) :

@staticmethod
def sendImage( image, catalogue, extraParameters = {}, waitForSave = True ) :
def sendImage( image, catalogue, extraParameters = {}, waitForSave = True, close = True ) :

if catalogue["directory"].getValue() and waitForSave :

# When the image has been received, the Catalogue will
# save it to disk on a background thread, and we need
# to wait for that to complete.
with GafferTest.ParallelAlgoTest.ExpectedUIThreadCall() :
GafferImageTest.DisplayTest.Driver.sendImage( image, GafferImage.Catalogue.displayDriverServer().portNumber(), extraParameters )
return GafferImageTest.DisplayTest.Driver.sendImage( image, GafferImage.Catalogue.displayDriverServer().portNumber(), extraParameters, close = close )

else :

GafferImageTest.DisplayTest.Driver.sendImage( image, GafferImage.Catalogue.displayDriverServer().portNumber(), extraParameters )
return GafferImageTest.DisplayTest.Driver.sendImage( image, GafferImage.Catalogue.displayDriverServer().portNumber(), extraParameters, close = close )

def testImages( self ) :

Expand Down Expand Up @@ -546,6 +546,43 @@ def testDeleteBeforeSaveCompletes( self ) :
self.sendImage( r["out"], c, waitForSave = False )
del c

def testDeleteBeforeSaveCompletesWithScriptVariables( self ) :

s = Gaffer.ScriptNode()
s["fileName"].setValue( "testDeleteBeforeSaveCompletesWithScriptVariables" )
self.assertEqual( s.context().substitute( "${script:name}" ), "testDeleteBeforeSaveCompletesWithScriptVariables" )

baseDirectory = os.path.join( self.temporaryDirectory(), "catalogue" )
# we don't expect to need to write here, but to ensure
# we didn't even try to do so we make it read only.
os.mkdir( baseDirectory )
os.chmod( baseDirectory, stat.S_IREAD )
directory = os.path.join( baseDirectory, "${script:name}", "images" )

s["c"] = GafferImage.Catalogue()
s["c"]["directory"].setValue( directory )

fullDirectory = s.context().substitute( s["c"]["directory"].getValue() )
self.assertNotEqual( directory, fullDirectory )
self.assertFalse( os.path.exists( directory ) )
self.assertFalse( os.path.exists( fullDirectory ) )

r = GafferImage.ImageReader()
r["fileName"].setValue( "${GAFFER_ROOT}/python/GafferImageTest/images/checker.exr" )

driver = self.sendImage( r["out"], s["c"], waitForSave = False, close = False )
# Simulate deletion by removing it from the script but keeping it alive.
# This would typically be fine, but we've setup the directory to require
# a script and then removed the script prior to closing the driver. Note
# we can't actually delete the catalogue yet or `driver.close()` would
# hang indefinitely waiting for an imageReceivedSignal.
c = s["c"]
s.removeChild( s["c"] )
driver.close()

self.assertFalse( os.path.exists( directory ) )
self.assertFalse( os.path.exists( fullDirectory ) )

def testNonWritableDirectory( self ) :

s = Gaffer.ScriptNode()
Expand Down
11 changes: 11 additions & 0 deletions src/GafferImage/Catalogue.cpp
Expand Up @@ -827,6 +827,17 @@ std::string Catalogue::generateFileName( const ImagePlug *image ) const
{
directory = script->context()->substitute( directory );
}
else if( Context::hasSubstitutions( directory ) )
{
// Its possible for a Catalogue to have been removed from its script
// and still receive an image. If it will attempt to save that image
// to a file which needed the script context to resolve properly, the
// saving will eventually error, so we return an empty string instead.
// Its likely this only occurs while the node is in the process of
// being deleted (perhaps inside python's garbage collector).
return "";
}

if( directory.empty() )
{
return "";
Expand Down

0 comments on commit 77cb07d

Please sign in to comment.