Remove volatile terminated flag to only use volatile termination reason

community/kernel/src/main/java/org/neo4j/kernel/api/KernelTransaction.java

@@ -113,16 +113,16 @@ interface CloseListener
     boolean isOpen();
 
     /**
-     * @return {@code true} if {@link #markForTermination(Status)} has been invoked, otherwise {@code false}.
+     * @return {@link Status} if {@link #markForTermination(Status)} has been invoked, otherwise {@code null}.
      */
-    Status shouldBeTerminated();
+    Status terminationReason();
 
     /**
      * Marks this transaction for termination, such that it cannot commit successfully and will try to be
      * terminated by having other methods throw a specific termination exception, as to sooner reach the assumed
      * point where {@link #close()} will be invoked.
      */
-    void markForTermination( Status typeOfReason );
+    void markForTermination( Status reason );
 
     /**
      * Register a {@link CloseListener} to be invoked after commit, but before transaction events "after" hooks

community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java

@@ -128,7 +128,7 @@ void assertOpen()
             throw new NotInTransactionException( "The statement has been closed." );
         }
 
-        Status terminationReason = transaction.shouldBeTerminated();
+        Status terminationReason = transaction.terminationReason();
         if ( terminationReason != null )
         {
             throw new TransactionTerminatedException( terminationReason );

community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java

@@ -172,8 +172,7 @@ TransactionType upgradeToSchemaTransaction() throws InvalidTransactionTypeKernel
     private StoreStatement storeStatement;
     private boolean closing, closed;
     private boolean failure, success;
-    private volatile boolean terminated;
-    private Status terminationReason;
+    private volatile Status terminationReason;
     // Some header information
     private long startTimeMillis;
     private long lastTransactionIdWhenStarted;
@@ -255,7 +254,7 @@ public KernelTransactionImplementation initialize( long lastCommittedTx, long la
     {
         this.locks = locksManager.newClient();
         this.terminationReason = null;
-        this.closing = closed = failure = success = terminated = false;
+        this.closing = closed = failure = success = false;
         this.transactionType = TransactionType.ANY;
         this.beforeHookInvoked = false;
         this.recordState.initialize( lastCommittedTx );
@@ -287,9 +286,9 @@ public void failure()
     }
 
     @Override
-    public Status shouldBeTerminated()
+    public Status terminationReason()
     {
-        return terminated ? terminationReason : null;
+        return terminationReason;
     }
 
     /**
@@ -317,7 +316,6 @@ public void markForTermination( Status reason )
             if ( stillSameTransaction && canBeTerminated() )
             {
                 failure = true;
-                terminated = true;
                 terminationReason = reason;
                 if ( txTerminationAwareLocks && locks != null )
                 {
@@ -490,7 +488,7 @@ public void close() throws TransactionFailureException
         closing = true;
         try
         {
-            if ( failure || !success || terminated )
+            if ( failure || !success || isTerminated() )
             {
                 rollback();
                 failOnNonExplicitRollbackIfNeeded( );
@@ -542,7 +540,7 @@ public void close() throws TransactionFailureException
      */
     private void failOnNonExplicitRollbackIfNeeded() throws TransactionFailureException
     {
-        if ( success && terminated )
+        if ( success && isTerminated() )
         {
             throw new TransactionTerminatedException( terminationReason );
         }
@@ -754,7 +752,6 @@ private void release()
         {
             locks.close();
             locks = null;
-            terminated = false;
             terminationReason = null;
             pool.release( this );
             if ( storeStatement != null )
@@ -776,7 +773,12 @@ private void release()
      */
     private boolean canBeTerminated()
     {
-        return !closed && !terminated;
+        return !closed && !isTerminated();
+    }
+
+    private boolean isTerminated()
+    {
+        return terminationReason != null;
     }
 
     public long lastTransactionTimestampWhenStarted()

community/kernel/src/main/java/org/neo4j/kernel/impl/core/ThreadToStatementContextBridge.java

@@ -71,7 +71,7 @@ private void assertInUnterminatedTransaction( TopLevelTransaction transaction )
         {
             throw new NotInTransactionException();
         }
-        Status terminationReason = transaction.getTransaction().shouldBeTerminated();
+        Status terminationReason = transaction.getTransaction().terminationReason();
         if ( terminationReason != null )
         {
             throw new TransactionTerminatedException( terminationReason );

community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelStatementTest.java

@@ -70,7 +70,7 @@ public void shouldCloseOpenedLabelScanReader() throws Exception
     public void shouldThrowTerminateExceptionWhenTransactionTerminated() throws Exception
     {
         KernelTransactionImplementation transaction = mock( KernelTransactionImplementation.class );
-        when( transaction.shouldBeTerminated() ).thenReturn( Status.General.UnknownFailure );
+        when( transaction.terminationReason() ).thenReturn( Status.General.UnknownFailure );
 
         KernelStatement statement = new KernelStatement(
             transaction, mock( IndexReaderFactory.class ),

community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionImplementationTest.java

@@ -59,6 +59,7 @@
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -176,7 +177,7 @@ public void shouldRollbackOnClosingSuccessfulButTerminatedTransaction() throws E
         {
             // WHEN
             transaction.markForTermination( Status.General.UnknownFailure );
-            assertEquals( Status.General.UnknownFailure, transaction.shouldBeTerminated() );
+            assertEquals( Status.General.UnknownFailure, transaction.terminationReason() );
         }
 
         // THEN
@@ -193,7 +194,7 @@ public void shouldRollbackOnClosingTerminatedButSuccessfulTransaction() throws E
         transaction.markForTermination( Status.General.UnknownFailure );
         transaction.success();
 
-        assertEquals( Status.General.UnknownFailure, transaction.shouldBeTerminated() );
+        assertEquals( Status.General.UnknownFailure, transaction.terminationReason() );
 
         try
         {
@@ -219,7 +220,7 @@ public void shouldNotDowngradeFailureState() throws Exception
             // WHEN
             transaction.markForTermination( Status.General.UnknownFailure );
             transaction.failure();
-            assertEquals( Status.General.UnknownFailure, transaction.shouldBeTerminated() );
+            assertEquals( Status.General.UnknownFailure, transaction.terminationReason() );
         }
 
         // THEN
@@ -433,7 +434,7 @@ public void markForTerminationNotInitializedTransaction()
 
         transaction.markForTermination( Status.General.UnknownFailure );
 
-        assertEquals( Status.General.UnknownFailure, transaction.shouldBeTerminated() );
+        assertEquals( Status.General.UnknownFailure, transaction.terminationReason() );
     }
 
     @Test
@@ -447,7 +448,7 @@ public void markForTerminationInitializedTransaction()
 
         transaction.markForTermination( Status.General.UnknownFailure );
 
-        assertEquals( Status.General.UnknownFailure, transaction.shouldBeTerminated() );
+        assertEquals( Status.General.UnknownFailure, transaction.terminationReason() );
         verify( client ).stop();
     }
 
@@ -464,7 +465,7 @@ public void markForTerminationTerminatedTransaction()
         transaction.markForTermination( Status.General.UnknownFailure );
         transaction.markForTermination( Status.General.UnknownFailure );
 
-        assertEquals( Status.General.UnknownFailure, transaction.shouldBeTerminated() );
+        assertEquals( Status.General.UnknownFailure, transaction.terminationReason() );
         verify( client ).stop();
         verify( transactionMonitor ).transactionTerminated();
     }
@@ -567,6 +568,31 @@ public void txMarkedForBothSuccessAndFailureThrowsOnClose()
         }
     }
 
+    @Test
+    public void initializedTransactionShouldHaveNoTerminationReason() throws Exception
+    {
+        KernelTransactionImplementation tx = newInitializedTransaction();
+        assertNull( tx.terminationReason() );
+    }
+
+    @Test
+    public void shouldReportCorrectTerminationReason() throws Exception
+    {
+        Status status = Status.Transaction.Terminated;
+        KernelTransactionImplementation tx = newInitializedTransaction();
+        tx.markForTermination( status );
+        assertSame( status, tx.terminationReason() );
+    }
+
+    @Test
+    public void closedTransactionShouldHaveNoTerminationReason() throws Exception
+    {
+        KernelTransactionImplementation tx = newInitializedTransaction();
+        tx.markForTermination( Status.Transaction.Terminated );
+        tx.close();
+        assertNull( tx.terminationReason() );
+    }
+
     private final NeoStores neoStores = mock( NeoStores.class );
     private final MetaDataStore metaDataStore = mock( MetaDataStore.class );
     private final TransactionHooks hooks = new TransactionHooks();

community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionTerminationTest.java

@@ -84,7 +84,7 @@ public void accept( TestKernelTransaction tx )
                     public void accept( TestKernelTransaction tx )
                     {
                         close( tx );
-                        assertNull( tx.shouldBeTerminated() );
+                        assertNull( tx.terminationReason() );
                         tx.initialize();
                     }
                 }
@@ -422,13 +422,13 @@ void assertRolledBack()
 
         void assertTerminated()
         {
-            assertEquals( Status.Transaction.MarkedAsFailed, shouldBeTerminated() );
+            assertEquals( Status.Transaction.MarkedAsFailed, terminationReason() );
             assertTrue( monitor.terminated );
         }
 
         void assertNotTerminated()
         {
-            assertNull( shouldBeTerminated() );
+            assertNull( terminationReason() );
             assertFalse( monitor.terminated );
         }
     }

community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionsTest.java

@@ -112,7 +112,7 @@ public void shouldDisposeTransactionsWhenAsked() throws Exception
         assertThat( postDispose, not( equalTo( first ) ) );
         assertThat( postDispose, not( equalTo( second ) ) );
 
-        assertTrue( leftOpen.shouldBeTerminated() != null );
+        assertTrue( leftOpen.terminationReason() != null );
     }
 
     @Test

community/kernel/src/test/java/org/neo4j/kernel/impl/api/LockingStatementOperationsTest.java

@@ -79,7 +79,7 @@ public LockingStatementOperationsTest()
                 entityReadOps, entityWriteOps, schemaReadOps, schemaWriteOps, schemaStateOps
         );
 
-        when( transaction.shouldBeTerminated() ).thenReturn( null );
+        when( transaction.terminationReason() ).thenReturn( null );
     }
 
     @Test

community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java

@@ -50,7 +50,6 @@
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
-
 import static org.neo4j.kernel.impl.api.StatementOperationsTestHelper.mockedParts;
 import static org.neo4j.kernel.impl.api.StatementOperationsTestHelper.mockedState;
 import static org.neo4j.kernel.impl.store.SchemaStorage.IndexRuleKind.CONSTRAINT;
@@ -199,7 +198,7 @@ public boolean isOpen()
                 }
 
                 @Override
-                public Status shouldBeTerminated()
+                public Status terminationReason()
                 {
                     return null;
                 }