Merge pull request #277 from andrewkaufman/rootAttrCrash

 Fixing seg fault when attempting to read a non-existant attribute several times

Author: ldmoser

src/IECore/SceneCache.cpp

@@ -535,6 +535,7 @@ class SceneCache::ReaderImplementation : public SceneCache::Implementation
 			}
 			if ( !it.first->second )
 			{
+				m_attributeSampleTimes.erase( it.first );
 				throw Exception( ( boost::format( "No samples for attribute %s available" ) % name.value() ).str() );
 			}
 			return *(it.first->second);

test/IECore/SceneCacheThreadingTest.cpp

@@ -34,8 +34,11 @@
 
 #include <vector>
 #include <iostream>
+
 #include "tbb/tbb.h"
+
 #include "IECore/SharedSceneInterfaces.h"
+
 #include "SceneCacheThreadingTest.h"
 
 using namespace boost;
@@ -52,61 +55,72 @@ struct SceneCacheThreadingTest
 	{
 		public :
 
-			TestSceneCache() : m_errors( 0 )
+			TestSceneCache( const char *attribute ) : m_errors( 0 ), m_attribute( attribute )
 			{
 			}
-
-			void operator() ( int instance ) const
+			
+			TestSceneCache( TestSceneCache &that, tbb::split ) : m_errors( 0 ), m_attribute( that.m_attribute )
 			{
-				for ( size_t i = 0; i < 1000; i++ )
+			}
+			
+			void operator()( const blocked_range<size_t> &r ) const
+			{
+				for ( size_t i = r.begin(); i != r.end(); ++i )
 				{
-					if ( (instance + i) % 7 == 0 )
-					{
-						SharedSceneInterfaces::clear();
-					}
-					ConstSceneInterfacePtr scene = SharedSceneInterfaces::get("test/IECore/data/sccFiles/attributeAtRoot.scc");
-					try 
+					for ( size_t j = 0; j < 1000; j++ )
 					{
-						scene->readAttribute("w", 0);
-					}
-					catch( Exception e )
-					{
-						m_errors++;
+						if ( ( i + j ) % 7 == 0 )
+						{
+							SharedSceneInterfaces::clear();
+						}
+
+						ConstSceneInterfacePtr scene = SharedSceneInterfaces::get("test/IECore/data/sccFiles/attributeAtRoot.scc");
+
+						try 
+						{
+							scene->readAttribute( m_attribute, 0 );
+						}
+						catch( Exception &e )
+						{
+							m_errors++;
+						}
 					}
 				}
 			}
-
+			
+			void join( const TestSceneCache &that )
+			{
+				m_errors += that.m_errors;
+			}
+			
 			size_t errors() const
 			{
 				return m_errors;
 			}
 
 		private :
 			mutable size_t m_errors;
-			SceneInterface::Path m_attributePath;
+			SceneInterface::Name m_attribute;
 	};
 
 	void testAttributeRead()
 	{
-		const int numThreads = 100;
-
-		TestSceneCache task;
-		std::vector< tbb::tbb_thread *> threads;
-
-		for ( int i = 0; i < numThreads; i++ )
-		{
-			threads.push_back( new tbb::tbb_thread( task, i ) );
-		}
-		for ( int i = 0; i < numThreads; i++ )
-		{
-			threads[i]->join();
-		}
-		for ( int i = 0; i < numThreads; i++ )
-		{
-			delete threads[i];
-		}
-		BOOST_CHECK( task.errors() == 0 );
-
+		task_scheduler_init scheduler( 100 );
+		
+		TestSceneCache task( "w" );
+		
+		parallel_reduce( blocked_range<size_t>( 0, 100 ), task );
+ 		BOOST_CHECK( task.errors() == 0 );
+	}
+	
+	void testFakeAttributeRead()
+	{
+		task_scheduler_init scheduler( 100 );
+		
+		TestSceneCache task( "fake" );
+		
+		parallel_reduce( blocked_range<size_t>( 0, 100 ), task );
+ 		BOOST_CHECK( task.errors() == 100000 );
 	}
 
 };
@@ -119,6 +133,7 @@ struct SceneCacheThreadingTestSuite : public boost::unit_test::test_suite
 		boost::shared_ptr<SceneCacheThreadingTest> instance( new SceneCacheThreadingTest() );
 
 		add( BOOST_CLASS_TEST_CASE( &SceneCacheThreadingTest::testAttributeRead, instance ) );
+		add( BOOST_CLASS_TEST_CASE( &SceneCacheThreadingTest::testFakeAttributeRead, instance ) );
 	}
 };