diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java index c2e0bb8f06337..c6503a8fb718b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java @@ -215,8 +215,8 @@ public KernelTransactionImplementation( StatementOperationContainer operationCon * Reset this transaction to a vanilla state, turning it into a logically new transaction. */ public KernelTransactionImplementation initialize( - long lastCommittedTx, long lastTimeStamp, StatementLocks statementLocks, Type type, SecurityContext securityContext, - long transactionTimeout ) + long lastCommittedTx, long lastTimeStamp, StatementLocks statementLocks, Type type, + SecurityContext frozenSecurityContext, long transactionTimeout ) { this.type = type; this.statementLocks = statementLocks; @@ -229,7 +229,7 @@ public KernelTransactionImplementation initialize( this.lastTransactionTimestampWhenStarted = lastTimeStamp; this.transactionEvent = tracer.beginTransaction(); assert transactionEvent != null : "transactionEvent was null!"; - this.securityContext = securityContext.freeze(); + this.securityContext = frozenSecurityContext; this.transactionId = NOT_COMMITTED_TRANSACTION_ID; this.commitTime = NOT_COMMITTED_TRANSACTION_COMMIT_TIME; this.currentTransactionOperations = timeoutMillis > 0 ? operationContainer.guardedParts() : operationContainer.nonGuarderParts(); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java index 32f8d254bdba9..0251ea801cf5a 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java @@ -157,6 +157,7 @@ public KernelTransactionImplementation newInstance() public KernelTransaction newInstance( KernelTransaction.Type type, SecurityContext securityContext, long timeout ) { assertCurrentThreadIsNotBlockingNewTransactions(); + SecurityContext frozenSecurityContext = securityContext.freeze(); newTransactionsLock.readLock().lock(); try { @@ -165,7 +166,7 @@ public KernelTransaction newInstance( KernelTransaction.Type type, SecurityConte KernelTransactionImplementation tx = localTxPool.acquire(); StatementLocks statementLocks = statementLocksFactory.newInstance(); tx.initialize( lastCommittedTransaction.transactionId(), - lastCommittedTransaction.commitTimestamp(), statementLocks, type, securityContext, timeout ); + lastCommittedTransaction.commitTimestamp(), statementLocks, type, frozenSecurityContext, timeout ); return tx; } finally @@ -297,5 +298,4 @@ private void assertCurrentThreadIsNotBlockingNewTransactions() "Thread that is blocking new transactions from starting can't start new transaction" ); } } - } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionsTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionsTest.java index 73163d97bc08f..7dcd4efd54a37 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionsTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionsTest.java @@ -32,11 +32,13 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReferenceArray; +import org.neo4j.graphdb.security.AuthorizationExpiredException; import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.KernelTransactionHandle; import org.neo4j.kernel.api.exceptions.Status; import org.neo4j.kernel.api.exceptions.TransactionFailureException; import org.neo4j.kernel.api.security.AnonymousContext; +import org.neo4j.kernel.api.security.SecurityContext; import org.neo4j.kernel.impl.api.state.ConstraintIndexCreator; import org.neo4j.kernel.impl.factory.AccessCapability; import org.neo4j.kernel.impl.factory.CanWrite; @@ -69,9 +71,8 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.locks.LockSupport.parkNanos; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.empty; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -90,6 +91,7 @@ import static org.neo4j.kernel.api.KernelTransaction.Type.explicit; import static org.neo4j.kernel.api.security.SecurityContext.AUTH_DISABLED; import static org.neo4j.kernel.impl.transaction.TransactionHeaderInformationFactory.DEFAULT; +import static org.neo4j.test.assertion.Assert.assertException; public class KernelTransactionsTest { @@ -374,7 +376,19 @@ public void unblockNewTransactionsFromWrongThreadThrows() throws Exception assertNotNull( txOpener.get( 2, TimeUnit.SECONDS ) ); } - private static void startAndCloseTransaction( KernelTransactions kernelTransactions ) + @Test + public void shouldNotLeakTransactionOnSecurityContextFreezeFailure() throws Exception { + KernelTransactions kernelTransactions = newKernelTransactions(); + SecurityContext securityContext = mock(SecurityContext.class); + when(securityContext.freeze()).thenThrow(new AuthorizationExpiredException("Freeze failed.")); + + assertException(() -> kernelTransactions.newInstance(KernelTransaction.Type.explicit, securityContext, 0L), + AuthorizationExpiredException.class, "Freeze failed."); + + assertThat("We should not have any transaction", kernelTransactions.activeTransactions(), is(empty())); + } + + private static void startAndCloseTransaction(KernelTransactions kernelTransactions) { try { @@ -490,25 +504,21 @@ private static KernelTransactionHandle newHandle( KernelTransaction tx ) return new TestKernelTransactionHandle( tx ); } - protected KernelTransaction getKernelTransaction( KernelTransactions transactions ) + private KernelTransaction getKernelTransaction( KernelTransactions transactions ) { return transactions.newInstance( KernelTransaction.Type.implicit, AnonymousContext.none(), 0L ); } private static class TestKernelTransactions extends KernelTransactions { - public TestKernelTransactions( StatementLocksFactory statementLocksFactory, - ConstraintIndexCreator constraintIndexCreator, - StatementOperationContainer statementOperationsContianer, - SchemaWriteGuard schemaWriteGuard, - TransactionHeaderInformationFactory txHeaderFactory, - TransactionCommitProcess transactionCommitProcess, - IndexConfigStore indexConfigStore, - LegacyIndexProviderLookup legacyIndexProviderLookup, TransactionHooks hooks, - TransactionMonitor transactionMonitor, LifeSupport dataSourceLife, - Tracers tracers, StorageEngine storageEngine, Procedures procedures, - TransactionIdStore transactionIdStore, Clock clock, - AccessCapability accessCapability ) + TestKernelTransactions( StatementLocksFactory statementLocksFactory, + ConstraintIndexCreator constraintIndexCreator, StatementOperationContainer statementOperationsContianer, + SchemaWriteGuard schemaWriteGuard, TransactionHeaderInformationFactory txHeaderFactory, + TransactionCommitProcess transactionCommitProcess, IndexConfigStore indexConfigStore, + LegacyIndexProviderLookup legacyIndexProviderLookup, TransactionHooks hooks, + TransactionMonitor transactionMonitor, LifeSupport dataSourceLife, Tracers tracers, + StorageEngine storageEngine, Procedures procedures, TransactionIdStore transactionIdStore, Clock clock, + AccessCapability accessCapability ) { super( statementLocksFactory, constraintIndexCreator, statementOperationsContianer, schemaWriteGuard, txHeaderFactory, transactionCommitProcess, indexConfigStore, legacyIndexProviderLookup, hooks,