Skip to content

Commit 78cf1e3

Browse files
committed
Forget auto-read managing handler when it is dequeued
Make sure `InboundMessageDispatcher` does not hold on to the last auto-read management handler after it is removed from the queue of all handlers.
1 parent 5c968ca commit 78cf1e3

File tree

2 files changed

+72
-9
lines changed

2 files changed

+72
-9
lines changed

driver/src/main/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcher.java

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void handleAckFailureMessage()
119119
public void handleSuccessMessage( Map<String,Value> meta )
120120
{
121121
log.debug( "S: SUCCESS %s", meta );
122-
ResponseHandler handler = handlers.remove();
122+
ResponseHandler handler = removeHandler();
123123
handler.onSuccess( meta );
124124
}
125125

@@ -152,7 +152,7 @@ public void handleFailureMessage( String code, String message )
152152
// try to write ACK_FAILURE before notifying the next response handler
153153
ackFailureIfNeeded();
154154

155-
ResponseHandler handler = handlers.remove();
155+
ResponseHandler handler = removeHandler();
156156
handler.onFailure( currentError );
157157
}
158158

@@ -161,7 +161,7 @@ public void handleIgnoredMessage()
161161
{
162162
log.debug( "S: IGNORED" );
163163

164-
ResponseHandler handler = handlers.remove();
164+
ResponseHandler handler = removeHandler();
165165

166166
Throwable error;
167167
if ( currentError != null )
@@ -189,7 +189,7 @@ public void handleFatalError( Throwable error )
189189

190190
while ( !handlers.isEmpty() )
191191
{
192-
ResponseHandler handler = handlers.remove();
192+
ResponseHandler handler = removeHandler();
193193
handler.onFailure( currentError );
194194
}
195195
}
@@ -245,6 +245,14 @@ public boolean isAckFailureMuted()
245245
return ackFailureMuted;
246246
}
247247

248+
/**
249+
* <b>Visible for testing</b>
250+
*/
251+
AutoReadManagingResponseHandler autoReadManagingHandler()
252+
{
253+
return autoReadManagingHandler;
254+
}
255+
248256
private void ackFailureIfNeeded()
249257
{
250258
if ( !ackFailureMuted )
@@ -254,6 +262,18 @@ private void ackFailureIfNeeded()
254262
}
255263
}
256264

265+
private ResponseHandler removeHandler()
266+
{
267+
ResponseHandler handler = handlers.remove();
268+
if ( autoReadManagingHandler == handler )
269+
{
270+
// handler that is being removed is the auto-read managing handler
271+
// make sure this dispatcher does not hold on to a removed handler
272+
autoReadManagingHandler = null;
273+
}
274+
return handler;
275+
}
276+
257277
private void updateAutoReadManagingHandlerIfNeeded( ResponseHandler handler )
258278
{
259279
if ( handler instanceof AutoReadManagingResponseHandler )
@@ -263,7 +283,7 @@ private void updateAutoReadManagingHandlerIfNeeded( ResponseHandler handler )
263283
// there already exists a handler that manages channel's auto-read
264284
// make it stop because new managing handler is being added and there should only be a single such handler
265285
autoReadManagingHandler.disableAutoReadManagement();
266-
// restore the original auto-read value
286+
// restore the default value of auto-read
267287
channel.config().setAutoRead( true );
268288
}
269289
autoReadManagingHandler = (AutoReadManagingResponseHandler) handler;

driver/src/test/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcherTest.java

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,44 @@ public void shouldKeepSingleAutoReadManagingHandler()
465465
inOrder.verify( handler3, never() ).disableAutoReadManagement();
466466
}
467467

468+
@Test
469+
public void shouldKeepTrackOfAutoReadManagingHandler()
470+
{
471+
InboundMessageDispatcher dispatcher = newDispatcher();
472+
473+
AutoReadManagingResponseHandler handler1 = mock( AutoReadManagingResponseHandler.class );
474+
AutoReadManagingResponseHandler handler2 = mock( AutoReadManagingResponseHandler.class );
475+
476+
assertNull( dispatcher.autoReadManagingHandler() );
477+
478+
dispatcher.enqueue( handler1 );
479+
assertEquals( handler1, dispatcher.autoReadManagingHandler() );
480+
481+
dispatcher.enqueue( handler2 );
482+
assertEquals( handler2, dispatcher.autoReadManagingHandler() );
483+
}
484+
485+
@Test
486+
public void shouldForgetAutoReadManagingHandlerWhenItIsRemoved()
487+
{
488+
InboundMessageDispatcher dispatcher = newDispatcher();
489+
490+
ResponseHandler handler1 = mock( ResponseHandler.class );
491+
ResponseHandler handler2 = mock( ResponseHandler.class );
492+
AutoReadManagingResponseHandler handler3 = mock( AutoReadManagingResponseHandler.class );
493+
494+
dispatcher.enqueue( handler1 );
495+
dispatcher.enqueue( handler2 );
496+
dispatcher.enqueue( handler3 );
497+
assertEquals( handler3, dispatcher.autoReadManagingHandler() );
498+
499+
dispatcher.handleSuccessMessage( emptyMap() );
500+
dispatcher.handleSuccessMessage( emptyMap() );
501+
dispatcher.handleSuccessMessage( emptyMap() );
502+
503+
assertNull( dispatcher.autoReadManagingHandler() );
504+
}
505+
468506
private static void verifyFailure( ResponseHandler handler )
469507
{
470508
ArgumentCaptor<Neo4jException> captor = ArgumentCaptor.forClass( Neo4jException.class );
@@ -475,14 +513,19 @@ private static void verifyFailure( ResponseHandler handler )
475513

476514
private static InboundMessageDispatcher newDispatcher()
477515
{
478-
Channel channel = mock( Channel.class );
479-
ChannelConfig channelConfig = mock( ChannelConfig.class );
480-
when( channel.config() ).thenReturn( channelConfig );
481-
return newDispatcher( channel );
516+
return newDispatcher( newChannelMock() );
482517
}
483518

484519
private static InboundMessageDispatcher newDispatcher( Channel channel )
485520
{
486521
return new InboundMessageDispatcher( channel, DEV_NULL_LOGGING );
487522
}
523+
524+
private static Channel newChannelMock()
525+
{
526+
Channel channel = mock( Channel.class );
527+
ChannelConfig channelConfig = mock( ChannelConfig.class );
528+
when( channel.config() ).thenReturn( channelConfig );
529+
return channel;
530+
}
488531
}

0 commit comments

Comments
 (0)