From a65241de67f8ebccd233e107b200c7a0f1469c08 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Mon, 15 Jan 2018 15:25:00 +0100 Subject: [PATCH] Fix a few cases where a static initialiser references a subclass. These situations can lead to classloader deadlocks, if the super-class and the sub-class ends up being loaded concurrently in two different threads. Not all cases are fixed: some a locked in place by being public API, and another was left be due to unpredictable consequences for forward merging. --- .../admin/AdminCommandSection.java | 4 +-- .../java/org/neo4j/helpers/FutureAdapter.java | 21 +++++++++++- .../org/neo4j/backup/BackupProtocolTest.java | 5 +-- .../src/main/java/org/neo4j/com/Response.java | 8 ----- .../main/java/org/neo4j/com/Responses.java | 34 +++++++++++++++++++ .../neo4j/kernel/ha/com/slave/SlaveImpl.java | 3 +- .../neo4j/ha/upgrade/MasterClientTest.java | 3 +- .../kernel/ha/SlaveUpdatePullerTest.java | 7 ++-- 8 files changed, 66 insertions(+), 19 deletions(-) create mode 100644 enterprise/com/src/main/java/org/neo4j/com/Responses.java diff --git a/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminCommandSection.java b/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminCommandSection.java index 7ff8bdceb5d33..654b1f036f7dc 100644 --- a/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminCommandSection.java +++ b/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminCommandSection.java @@ -26,14 +26,12 @@ public abstract class AdminCommandSection { - private static final AdminCommandSection GENERAL = new GeneralSection(); - @Nonnull public abstract String printable(); public static AdminCommandSection general() { - return GENERAL; + return new GeneralSection(); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/helpers/FutureAdapter.java b/community/kernel/src/main/java/org/neo4j/helpers/FutureAdapter.java index 8c9b31f8b9d05..dfaf4399fcbe9 100644 --- a/community/kernel/src/main/java/org/neo4j/helpers/FutureAdapter.java +++ b/community/kernel/src/main/java/org/neo4j/helpers/FutureAdapter.java @@ -76,7 +76,26 @@ public static Present present( T value ) return new Present<>( value ); } - public static final Future VOID = new Present<>( null ); + public static final Future VOID = new FutureAdapter() + { + @Override + public boolean isDone() + { + return true; + } + + @Override + public Void get() + { + return null; + } + + @Override + public Void get( long timeout, TimeUnit unit ) + { + return null; + } + }; public static Future latchGuardedValue( final Supplier supplier, final CountDownLatch guardedByLatch, final String jobDescription ) diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolTest.java b/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolTest.java index 445327458d9f7..abe0b5e15b726 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolTest.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolTest.java @@ -24,6 +24,7 @@ import org.neo4j.backup.BackupClient.BackupRequestType; import org.neo4j.com.RequestContext; import org.neo4j.com.Response; +import org.neo4j.com.Responses; import org.neo4j.com.TargetCaller; import org.neo4j.com.monitor.RequestMonitor; import org.neo4j.com.storecopy.ResponseUnpacker; @@ -73,7 +74,7 @@ public void shouldHandleNoForensicsSpecifiedInFullBackupRequest() throws Excepti private void shouldGatherForensicsInFullBackupRequest( boolean forensics ) throws Exception { // GIVEN - Response response = Response.EMPTY; + Response response = Responses.empty(); StoreId storeId = response.getStoreId(); String host = "localhost"; int port = BackupServer.DEFAULT_PORT; @@ -111,7 +112,7 @@ public Response fullBackup( StoreWriter writer, boolean forensics ) { this.receivedForensics = forensics; writer.close(); - return Response.EMPTY; + return Responses.empty(); } @Override diff --git a/enterprise/com/src/main/java/org/neo4j/com/Response.java b/enterprise/com/src/main/java/org/neo4j/com/Response.java index f09f2fc76ae2d..fdf5718e88a09 100644 --- a/enterprise/com/src/main/java/org/neo4j/com/Response.java +++ b/enterprise/com/src/main/java/org/neo4j/com/Response.java @@ -59,12 +59,6 @@ public void close() releaser.release(); } - @SuppressWarnings( "unchecked" ) - public static Response empty() - { - return (Response) EMPTY; - } - public abstract void accept( Handler handler ) throws Exception; /** @@ -93,6 +87,4 @@ public interface Handler Visitor transactions(); } - public static final Response EMPTY = new TransactionObligationResponse<>( null, StoreId.DEFAULT, - -1, ResourceReleaser.NO_OP ); } diff --git a/enterprise/com/src/main/java/org/neo4j/com/Responses.java b/enterprise/com/src/main/java/org/neo4j/com/Responses.java new file mode 100644 index 0000000000000..e79e1fa831c8a --- /dev/null +++ b/enterprise/com/src/main/java/org/neo4j/com/Responses.java @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package org.neo4j.com; + +import org.neo4j.kernel.impl.store.StoreId; + +public class Responses +{ + private static final Response EMPTY = + new TransactionObligationResponse<>( null, StoreId.DEFAULT, -1, ResourceReleaser.NO_OP );; + + @SuppressWarnings( "unchecked" ) + public static Response empty() + { + return (Response) EMPTY; + } +} diff --git a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/com/slave/SlaveImpl.java b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/com/slave/SlaveImpl.java index da66e745cdb87..cc174bf54ff87 100644 --- a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/com/slave/SlaveImpl.java +++ b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/com/slave/SlaveImpl.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.ha.com.slave; import org.neo4j.com.Response; +import org.neo4j.com.Responses; import org.neo4j.com.storecopy.TransactionObligationFulfiller; import org.neo4j.helpers.Exceptions; import org.neo4j.kernel.ha.com.master.Slave; @@ -44,7 +45,7 @@ public Response pullUpdates( long upToAndIncludingTxId ) { throw Exceptions.launderedException( e ); } - return Response.EMPTY; + return Responses.empty(); } @Override diff --git a/enterprise/ha/src/test/java/org/neo4j/ha/upgrade/MasterClientTest.java b/enterprise/ha/src/test/java/org/neo4j/ha/upgrade/MasterClientTest.java index bb7b90b4ee0e3..cb91df3dd54ec 100644 --- a/enterprise/ha/src/test/java/org/neo4j/ha/upgrade/MasterClientTest.java +++ b/enterprise/ha/src/test/java/org/neo4j/ha/upgrade/MasterClientTest.java @@ -28,6 +28,7 @@ import org.neo4j.com.RequestContext; import org.neo4j.com.ResourceReleaser; import org.neo4j.com.Response; +import org.neo4j.com.Responses; import org.neo4j.com.Server; import org.neo4j.com.StoreIdTestFactory; import org.neo4j.com.TransactionStream; @@ -155,7 +156,7 @@ public void endLockSessionDoesNotUnpackResponse() throws Throwable ResponseUnpacker responseUnpacker = mock( ResponseUnpacker.class ); MasterImpl.SPI masterImplSPI = MasterImplTest.mockedSpi( storeId ); when( masterImplSPI.packTransactionObligationResponse( any( RequestContext.class ), Matchers.anyObject() ) ) - .thenReturn( Response.empty() ); + .thenReturn( Responses.empty() ); when( masterImplSPI.getTransactionChecksum( anyLong() ) ).thenReturn( txChecksum ); newMasterServer( masterImplSPI ); diff --git a/enterprise/ha/src/test/java/org/neo4j/kernel/ha/SlaveUpdatePullerTest.java b/enterprise/ha/src/test/java/org/neo4j/kernel/ha/SlaveUpdatePullerTest.java index 186446884edbf..66455734c0ef7 100644 --- a/enterprise/ha/src/test/java/org/neo4j/kernel/ha/SlaveUpdatePullerTest.java +++ b/enterprise/ha/src/test/java/org/neo4j/kernel/ha/SlaveUpdatePullerTest.java @@ -36,6 +36,7 @@ import org.neo4j.com.ComException; import org.neo4j.com.RequestContext; import org.neo4j.com.Response; +import org.neo4j.com.Responses; import org.neo4j.kernel.AvailabilityGuard; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.ha.UpdatePuller.Condition; @@ -237,7 +238,7 @@ public void shouldCopeWithHardExceptionsLikeOutOfMemory() throws Exception OutOfMemoryError oom = new OutOfMemoryError(); when( master.pullUpdates( any( RequestContext.class ) ) ) .thenThrow( oom ) - .thenReturn( Response.EMPTY ); + .thenReturn( Responses.empty() ); // WHEN making the first pull updatePuller.pullUpdates(); @@ -265,7 +266,7 @@ public void shouldCapExcessiveComExceptionLogging() throws Exception logProvider.assertContainsThrowablesMatching( 0, repeat( new ComException(), SlaveUpdatePuller.LOG_CAP ) ); // And we should be able to recover afterwards - updatePullStubbing.thenReturn( Response.EMPTY ).thenThrow( new ComException() ); + updatePullStubbing.thenReturn( Responses.empty() ).thenThrow( new ComException() ); updatePuller.pullUpdates(); // This one will succeed and unlock the circuit breaker updatePuller.pullUpdates(); // And then we log another exception @@ -298,7 +299,7 @@ public void shouldCapExcessiveInvalidEpochExceptionLogging() throws Exception repeat( new InvalidEpochException( 2, 1 ), SlaveUpdatePuller.LOG_CAP ) ); // And we should be able to recover afterwards - updatePullStubbing.thenReturn( Response.EMPTY ).thenThrow( new InvalidEpochException( 2, 1 ) ); + updatePullStubbing.thenReturn( Responses.empty() ).thenThrow( new InvalidEpochException( 2, 1 ) ); updatePuller.pullUpdates(); // This one will succeed and unlock the circuit breaker updatePuller.pullUpdates(); // And then we log another exception