Properly closes resources in some tests to not leave threads hanging …

…around
neo4j · Oct 13, 2016 · 88759d3 · 88759d3
1 parent a989c4d
commit 88759d3
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 30 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/DebugUtil.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/DebugUtil.java
@@ -110,10 +110,27 @@ public static Predicate<StackTraceElement> classAndMethodAre( final Class<?> cls
     }
 
     public static void dumpThreads( PrintStream out )
+    {
+        dumpThreads( out, true, true );
+    }
+
+    public static void dumpThreads( PrintStream out, boolean excludeSystemThreads, boolean excludeCallingThread )
     {
         for ( Map.Entry<Thread,StackTraceElement[]> stack : Thread.getAllStackTraces().entrySet() )
         {
-            out.println( new DebugUtil.CallStack( stack.getValue(), "Thread: " + stack.getKey() ) );
+            Thread thread = stack.getKey();
+            ThreadGroup group = thread.getThreadGroup();
+            if ( excludeSystemThreads && group != null && "system".equals( thread.getThreadGroup().getName() ) )
+            {
+                continue;
+            }
+            if ( excludeCallingThread && thread == Thread.currentThread() )
+            {
+                continue;
+            }
+
+            out.println( new CallStack( stack.getValue(), "Thread: " + stack.getKey() ) );
+            out.println();
         }
     }
 

diff --git a/...unity/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingNeoStores.java b/...unity/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingNeoStores.java
@@ -122,7 +122,18 @@ public BatchingNeoStores( FileSystemAbstraction fileSystem, File storeDir, Recor
         if ( alreadyContainsData( neoStores ) )
         {
             neoStores.close();
-            throw new IllegalStateException( storeDir + " already contains data, cannot do import here" );
+            IllegalStateException ise =
+                    new IllegalStateException( storeDir + " already contains data, cannot do import here" );
+            try
+            {
+                pageCache.close();
+            }
+            catch ( IOException e )
+            {
+                // Oddly enough we can't close the page cache, how to communicate this? Here we add as suppressed
+                ise.addSuppressed( e );
+            }
+            throw ise;
         }
         try
         {

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/locking/RWLockCompatibility.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/locking/RWLockCompatibility.java
@@ -199,6 +199,13 @@ public void testMultipleThreads() throws Exception
             File file = dumper.dumpState( locks, t1, t2, t3, t4 );
             throw new RuntimeException( "Failed, forensics information dumped to " + file.getAbsolutePath(), e );
         }
+        finally
+        {
+            t1.close();
+            t2.close();
+            t3.close();
+            t4.close();
+        }
     }
 
     public class StressThread extends Thread

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java
@@ -24,7 +24,6 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.RuleChain;
-
 import java.io.File;
 import java.io.IOException;
 import java.nio.ByteBuffer;
@@ -103,9 +102,9 @@ public class NeoStoresTest
 
     private final PageCacheRule pageCacheRule = new PageCacheRule();
     private final ExpectedException exception = ExpectedException.none();
-    private EphemeralFileSystemRule fs = new EphemeralFileSystemRule();
-    private TestDirectory dir = TestDirectory.testDirectory( fs.get() );
-    private NeoStoreDataSourceRule dsRule = new NeoStoreDataSourceRule();
+    private final EphemeralFileSystemRule fs = new EphemeralFileSystemRule();
+    private final TestDirectory dir = TestDirectory.testDirectory( fs.get() );
+    private final NeoStoreDataSourceRule dsRule = new NeoStoreDataSourceRule();
 
     @Rule
     public RuleChain ruleChain = RuleChain.outerRule( exception ).around( pageCacheRule )
@@ -177,7 +176,14 @@ public void impossibleToGetNotRequestedStore()
         exception.expectMessage(
                 "Specified store was not initialized. Please specify " + StoreType.META_DATA.name() +
                 " as one of the stores types that should be open to be able to use it." );
-        neoStores.getMetaDataStore();
+        try
+        {
+            neoStores.getMetaDataStore();
+        }
+        finally
+        {
+            neoStores.close();
+        }
     }
 
     @Test

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestGraphProperties.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestGraphProperties.java
@@ -262,7 +262,7 @@ public void testEquals()
     @Test
     public void shouldBeAbleToCreateLongGraphPropertyChainsAndReadTheCorrectNextPointerFromTheStore()
     {
-        GraphDatabaseService database = new TestGraphDatabaseFactory().newImpermanentDatabase();
+        GraphDatabaseService database = factory.newImpermanentDatabase();
 
         PropertyContainer graphProperties = properties( (GraphDatabaseAPI) database );
 
@@ -305,6 +305,7 @@ public void shouldBeAbleToCreateLongGraphPropertyChainsAndReadTheCorrectNextPoin
             assertEquals( "I", graphProperties.getProperty( "i" ) );
             tx.success();
         }
+        database.shutdown();
     }
 
     private static class State

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/counts/CountsRotationTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/counts/CountsRotationTest.java
@@ -178,6 +178,8 @@ public void rotationShouldNotCauseUnmappedFileProblem() throws IOException
 
         assertEquals( "Should perform at least 100 rotations.", rotations, Math.min( rotations, countStore.txId() - startTxId) );
         assertTrue( "Should perform more then 0 lookups without exceptions.", lookupsCounter.get() > 0 );
+
+        db.shutdown();
     }
 
     private static ThrowingFunction<CountsTracker,Void,RuntimeException> countStoreLookup(

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/lifecycle/GitHub1304Test.java b/community/kernel/src/test/java/org/neo4j/kernel/lifecycle/GitHub1304Test.java
@@ -47,5 +47,6 @@ public void givenBatchInserterWhenArrayPropertyUpdated4TimesThenShouldNotFail()
         }
 
         batchInserter.getNodeProperties( nodeId );   //fails here
+        batchInserter.shutdown();
     }
 }
diff --git a/...unity/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/PropertyEncoderStepTest.java b/...unity/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/PropertyEncoderStepTest.java
@@ -94,6 +94,7 @@ public void shouldGrowPropertyBlocksArrayProperly() throws Exception
         // THEN
         verify( downstream ).receive( anyLong(), any() );
         verifyNoMoreInteractions( control );
+        step.close();
     }
 
     private void awaitCompleted( Step<?> step, StageControl control ) throws InterruptedException

diff --git a/...nel/src/test/java/org/neo4j/unsafe/impl/batchimport/executor/DynamicTaskExecutorTest.java b/...nel/src/test/java/org/neo4j/unsafe/impl/batchimport/executor/DynamicTaskExecutorTest.java
@@ -212,6 +212,8 @@ public void shouldShutDownOnTaskFailureEvenIfOtherTasksArePending() throws Excep
         // THEN
         assertExceptionOnSubmit( executor, exception );
         executor.shutdown( SF_ABORT_QUEUED ); // call would block if the shutdown as part of failure doesn't complete properly
+
+        secondBlockingTask.latch.finish();
     }
 
     @Test
@@ -307,7 +309,7 @@ public void shouldRespectMaxProcessors() throws Exception
         executor.shutdown( SF_AWAIT_ALL_COMPLETED );
     }
 
-    @Repeat( times = 100 )
+    @Repeat( times = 10 )
     @Test
     public void shouldCopeWithConcurrentIncrementOfProcessorsAndShutdown() throws Throwable
     {

diff --git a/...rc/test/java/org/neo4j/unsafe/impl/batchimport/input/PerTypeRelationshipSplitterTest.java b/...rc/test/java/org/neo4j/unsafe/impl/batchimport/input/PerTypeRelationshipSplitterTest.java
@@ -75,6 +75,7 @@ public void shouldReturnTypesOneByOne() throws Exception
         }
 
         assertEquals( nodesOf( expected.iterator() ), all );
+        inputCache.close();
     }
 
     /**

diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/CsvInputTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/CsvInputTest.java
@@ -29,7 +29,6 @@
 import java.io.IOException;
 import java.io.StringReader;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.Set;
 
 import org.neo4j.csv.reader.CharReadable;
@@ -107,9 +106,11 @@ null, null, idType, config( COMMAS ), silentBadCollector( 0 ),
                         getRuntime().availableProcessors() );
 
         // WHEN/THEN
-        Iterator<InputNode> nodes = input.nodes().iterator();
-        assertNode( nodes.next(), 123L, properties( "name", "Mattias Persson" ), labels( "HACKER" ) );
-        assertFalse( nodes.hasNext() );
+        try ( InputIterator<InputNode> nodes = input.nodes().iterator() )
+        {
+            assertNode( nodes.next(), 123L, properties( "name", "Mattias Persson" ), labels( "HACKER" ) );
+            assertFalse( nodes.hasNext() );
+        }
     }
 
     @Test
@@ -130,9 +131,11 @@ public void shouldProvideRelationshipsFromCsvInput() throws Exception
                         getRuntime().availableProcessors() );
 
         // WHEN/THEN
-        Iterator<InputRelationship> relationships = input.relationships().iterator();
-        assertRelationship( relationships.next(), "node1", "node2", "KNOWS", properties( "since", 1234567L ) );
-        assertRelationship( relationships.next(), "node2", "node10", "HACKS", properties( "since", 987654L ) );
+        try ( InputIterator<InputRelationship> relationships = input.relationships().iterator() )
+        {
+            assertRelationship( relationships.next(), "node1", "node2", "KNOWS", properties( "since", 1234567L ) );
+            assertRelationship( relationships.next(), "node2", "node10", "HACKS", properties( "since", 987654L ) );
+        }
     }
 
     @Test
@@ -250,12 +253,14 @@ IdType.STRING, config( COMMAS ), silentBadCollector( 0 ),
                                     getRuntime().availableProcessors() );
 
         // WHEN iterating over them, THEN the expected data should come out
-        ResourceIterator<InputNode> nodes = input.nodes().iterator();
-        assertNode( nodes.next(), "1", properties( "name", "Jim", "kills", 10, "health", 100 ), labels() );
-        assertNode( nodes.next(), "2", properties( "name", "Abathur", "kills", 0, "health", 200 ), labels() );
-        assertNode( nodes.next(), "3", properties( "type", "zergling" ), labels() );
-        assertNode( nodes.next(), "4", properties( "type", "csv" ), labels() );
-        assertFalse( nodes.hasNext() );
+        try ( InputIterator<InputNode> nodes = input.nodes().iterator() )
+        {
+            assertNode( nodes.next(), "1", properties( "name", "Jim", "kills", 10, "health", 100 ), labels() );
+            assertNode( nodes.next(), "2", properties( "name", "Abathur", "kills", 0, "health", 200 ), labels() );
+            assertNode( nodes.next(), "3", properties( "type", "zergling" ), labels() );
+            assertNode( nodes.next(), "4", properties( "type", "csv" ), labels() );
+            assertFalse( nodes.hasNext() );
+        }
     }
 
     @Test
@@ -566,10 +571,12 @@ null, null, idType, config( COMMAS ),
                         silentBadCollector( 0 ), getRuntime().availableProcessors() );
 
         // WHEN/THEN
-        Iterator<InputNode> nodes = input.nodes().iterator();
-        assertNode( nodes.next(), group, 123L, properties( "name", "one" ), labels() );
-        assertNode( nodes.next(), group, 456L, properties( "name", "two" ), labels() );
-        assertFalse( nodes.hasNext() );
+        try ( InputIterator<InputNode> nodes = input.nodes().iterator() )
+        {
+            assertNode( nodes.next(), group, 123L, properties( "name", "one" ), labels() );
+            assertNode( nodes.next(), group, 456L, properties( "name", "two" ), labels() );
+            assertFalse( nodes.hasNext() );
+        }
     }
 
     @Test
@@ -592,10 +599,12 @@ idType, config( COMMAS ),
                         silentBadCollector( 0 ), getRuntime().availableProcessors() );
 
         // WHEN/THEN
-        Iterator<InputRelationship> relationships = input.relationships().iterator();
-        assertRelationship( relationships.next(), startNodeGroup, 123L, endNodeGroup, 234L, "TYPE", properties() );
-        assertRelationship( relationships.next(), startNodeGroup, 345L, endNodeGroup, 456L, "TYPE", properties() );
-        assertFalse( relationships.hasNext() );
+        try ( InputIterator<InputRelationship> relationships = input.relationships().iterator() )
+        {
+            assertRelationship( relationships.next(), startNodeGroup, 123L, endNodeGroup, 234L, "TYPE", properties() );
+            assertRelationship( relationships.next(), startNodeGroup, 345L, endNodeGroup, 456L, "TYPE", properties() );
+            assertFalse( relationships.hasNext() );
+        }
     }
 
     @Test

diff --git a/...rc/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/InputGroupsDeserializerTest.java b/...rc/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/InputGroupsDeserializerTest.java
@@ -20,6 +20,8 @@
 package org.neo4j.unsafe.impl.batchimport.input.csv;
 
 import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 
 import java.io.StringReader;
 import java.util.List;
@@ -32,6 +34,7 @@
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.neo4j.csv.reader.Readables.wrap;
@@ -69,6 +72,15 @@ public void shouldBeAbleToAskForSourceInformationEvenBetweenTwoSources() throws
                     @SuppressWarnings( "unchecked" )
                     InputEntityDeserializer<InputNode> result = mock( InputEntityDeserializer.class );
                     when( result.sourceDescription() ).thenReturn( String.valueOf( flips.get() ) );
+                    doAnswer( new Answer<Void>()
+                    {
+                        @Override
+                        public Void answer( InvocationOnMock invocation ) throws Throwable
+                        {
+                            stream.close();
+                            return null;
+                        }
+                    } ).when( result ).close();
                     return result;
                 }, Validators.<InputNode>emptyValidator(), InputNode.class );
         deserializerTestHack.set( deserializer );
@@ -78,6 +90,7 @@ public void shouldBeAbleToAskForSourceInformationEvenBetweenTwoSources() throws
 
         // THEN there should have been two data source flips
         assertEquals( 2, flips.get() );
+        deserializer.close();
     }
 
     private Configuration lowBufferSize( Configuration conf )