Skip to content

Commit

Permalink
Fix a few cases where a static initialiser references a subclass.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chrisvest committed Jan 15, 2018
1 parent aa1dac5 commit a65241d
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 19 deletions.
Expand Up @@ -26,14 +26,12 @@


public abstract class AdminCommandSection public abstract class AdminCommandSection
{ {
private static final AdminCommandSection GENERAL = new GeneralSection();

@Nonnull @Nonnull
public abstract String printable(); public abstract String printable();


public static AdminCommandSection general() public static AdminCommandSection general()
{ {
return GENERAL; return new GeneralSection();
} }


@Override @Override
Expand Down
Expand Up @@ -76,7 +76,26 @@ public static <T> Present<T> present( T value )
return new Present<>( value ); return new Present<>( value );
} }


public static final Future<Void> VOID = new Present<>( null ); public static final Future<Void> VOID = new FutureAdapter<Void>()
{
@Override
public boolean isDone()
{
return true;
}

@Override
public Void get()
{
return null;
}

@Override
public Void get( long timeout, TimeUnit unit )
{
return null;
}
};


public static <T> Future<T> latchGuardedValue( final Supplier<T> supplier, final CountDownLatch guardedByLatch, public static <T> Future<T> latchGuardedValue( final Supplier<T> supplier, final CountDownLatch guardedByLatch,
final String jobDescription ) final String jobDescription )
Expand Down
Expand Up @@ -24,6 +24,7 @@
import org.neo4j.backup.BackupClient.BackupRequestType; import org.neo4j.backup.BackupClient.BackupRequestType;
import org.neo4j.com.RequestContext; import org.neo4j.com.RequestContext;
import org.neo4j.com.Response; import org.neo4j.com.Response;
import org.neo4j.com.Responses;
import org.neo4j.com.TargetCaller; import org.neo4j.com.TargetCaller;
import org.neo4j.com.monitor.RequestMonitor; import org.neo4j.com.monitor.RequestMonitor;
import org.neo4j.com.storecopy.ResponseUnpacker; import org.neo4j.com.storecopy.ResponseUnpacker;
Expand Down Expand Up @@ -73,7 +74,7 @@ public void shouldHandleNoForensicsSpecifiedInFullBackupRequest() throws Excepti
private void shouldGatherForensicsInFullBackupRequest( boolean forensics ) throws Exception private void shouldGatherForensicsInFullBackupRequest( boolean forensics ) throws Exception
{ {
// GIVEN // GIVEN
Response<Void> response = Response.EMPTY; Response<Void> response = Responses.empty();
StoreId storeId = response.getStoreId(); StoreId storeId = response.getStoreId();
String host = "localhost"; String host = "localhost";
int port = BackupServer.DEFAULT_PORT; int port = BackupServer.DEFAULT_PORT;
Expand Down Expand Up @@ -111,7 +112,7 @@ public Response<Void> fullBackup( StoreWriter writer, boolean forensics )
{ {
this.receivedForensics = forensics; this.receivedForensics = forensics;
writer.close(); writer.close();
return Response.EMPTY; return Responses.empty();
} }


@Override @Override
Expand Down
8 changes: 0 additions & 8 deletions enterprise/com/src/main/java/org/neo4j/com/Response.java
Expand Up @@ -59,12 +59,6 @@ public void close()
releaser.release(); releaser.release();
} }


@SuppressWarnings( "unchecked" )
public static <T> Response<T> empty()
{
return (Response<T>) EMPTY;
}

public abstract void accept( Handler handler ) throws Exception; public abstract void accept( Handler handler ) throws Exception;


/** /**
Expand Down Expand Up @@ -93,6 +87,4 @@ public interface Handler
Visitor<CommittedTransactionRepresentation,Exception> transactions(); Visitor<CommittedTransactionRepresentation,Exception> transactions();
} }


public static final Response<Void> EMPTY = new TransactionObligationResponse<>( null, StoreId.DEFAULT,
-1, ResourceReleaser.NO_OP );
} }
34 changes: 34 additions & 0 deletions 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 <http://www.gnu.org/licenses/>.
*/
package org.neo4j.com;

import org.neo4j.kernel.impl.store.StoreId;

public class Responses
{
private static final Response<Void> EMPTY =
new TransactionObligationResponse<>( null, StoreId.DEFAULT, -1, ResourceReleaser.NO_OP );;

@SuppressWarnings( "unchecked" )
public static <T> Response<T> empty()
{
return (Response<T>) EMPTY;
}
}
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.kernel.ha.com.slave; package org.neo4j.kernel.ha.com.slave;


import org.neo4j.com.Response; import org.neo4j.com.Response;
import org.neo4j.com.Responses;
import org.neo4j.com.storecopy.TransactionObligationFulfiller; import org.neo4j.com.storecopy.TransactionObligationFulfiller;
import org.neo4j.helpers.Exceptions; import org.neo4j.helpers.Exceptions;
import org.neo4j.kernel.ha.com.master.Slave; import org.neo4j.kernel.ha.com.master.Slave;
Expand All @@ -44,7 +45,7 @@ public Response<Void> pullUpdates( long upToAndIncludingTxId )
{ {
throw Exceptions.launderedException( e ); throw Exceptions.launderedException( e );
} }
return Response.EMPTY; return Responses.empty();
} }


@Override @Override
Expand Down
Expand Up @@ -28,6 +28,7 @@
import org.neo4j.com.RequestContext; import org.neo4j.com.RequestContext;
import org.neo4j.com.ResourceReleaser; import org.neo4j.com.ResourceReleaser;
import org.neo4j.com.Response; import org.neo4j.com.Response;
import org.neo4j.com.Responses;
import org.neo4j.com.Server; import org.neo4j.com.Server;
import org.neo4j.com.StoreIdTestFactory; import org.neo4j.com.StoreIdTestFactory;
import org.neo4j.com.TransactionStream; import org.neo4j.com.TransactionStream;
Expand Down Expand Up @@ -155,7 +156,7 @@ public void endLockSessionDoesNotUnpackResponse() throws Throwable
ResponseUnpacker responseUnpacker = mock( ResponseUnpacker.class ); ResponseUnpacker responseUnpacker = mock( ResponseUnpacker.class );
MasterImpl.SPI masterImplSPI = MasterImplTest.mockedSpi( storeId ); MasterImpl.SPI masterImplSPI = MasterImplTest.mockedSpi( storeId );
when( masterImplSPI.packTransactionObligationResponse( any( RequestContext.class ), Matchers.anyObject() ) ) when( masterImplSPI.packTransactionObligationResponse( any( RequestContext.class ), Matchers.anyObject() ) )
.thenReturn( Response.empty() ); .thenReturn( Responses.empty() );
when( masterImplSPI.getTransactionChecksum( anyLong() ) ).thenReturn( txChecksum ); when( masterImplSPI.getTransactionChecksum( anyLong() ) ).thenReturn( txChecksum );


newMasterServer( masterImplSPI ); newMasterServer( masterImplSPI );
Expand Down
Expand Up @@ -36,6 +36,7 @@
import org.neo4j.com.ComException; import org.neo4j.com.ComException;
import org.neo4j.com.RequestContext; import org.neo4j.com.RequestContext;
import org.neo4j.com.Response; import org.neo4j.com.Response;
import org.neo4j.com.Responses;
import org.neo4j.kernel.AvailabilityGuard; import org.neo4j.kernel.AvailabilityGuard;
import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.ha.UpdatePuller.Condition; import org.neo4j.kernel.ha.UpdatePuller.Condition;
Expand Down Expand Up @@ -237,7 +238,7 @@ public void shouldCopeWithHardExceptionsLikeOutOfMemory() throws Exception
OutOfMemoryError oom = new OutOfMemoryError(); OutOfMemoryError oom = new OutOfMemoryError();
when( master.pullUpdates( any( RequestContext.class ) ) ) when( master.pullUpdates( any( RequestContext.class ) ) )
.thenThrow( oom ) .thenThrow( oom )
.thenReturn( Response.EMPTY ); .thenReturn( Responses.empty() );


// WHEN making the first pull // WHEN making the first pull
updatePuller.pullUpdates(); updatePuller.pullUpdates();
Expand Down Expand Up @@ -265,7 +266,7 @@ public void shouldCapExcessiveComExceptionLogging() throws Exception
logProvider.assertContainsThrowablesMatching( 0, repeat( new ComException(), SlaveUpdatePuller.LOG_CAP ) ); logProvider.assertContainsThrowablesMatching( 0, repeat( new ComException(), SlaveUpdatePuller.LOG_CAP ) );


// And we should be able to recover afterwards // 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(); // This one will succeed and unlock the circuit breaker
updatePuller.pullUpdates(); // And then we log another exception updatePuller.pullUpdates(); // And then we log another exception
Expand Down Expand Up @@ -298,7 +299,7 @@ public void shouldCapExcessiveInvalidEpochExceptionLogging() throws Exception
repeat( new InvalidEpochException( 2, 1 ), SlaveUpdatePuller.LOG_CAP ) ); repeat( new InvalidEpochException( 2, 1 ), SlaveUpdatePuller.LOG_CAP ) );


// And we should be able to recover afterwards // 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(); // This one will succeed and unlock the circuit breaker
updatePuller.pullUpdates(); // And then we log another exception updatePuller.pullUpdates(); // And then we log another exception
Expand Down

0 comments on commit a65241d

Please sign in to comment.