Skip to content

Commit af6b33f

Browse files
committed
PR 2402 added support for CompletableFuture which accidentally introduced a double registration bug in HttpJettySolrClient by calling onRequestQueued/onComplete twice with asyncListener.queuedListener/completeListener
This introduces three undesirable scenarios: Potential deadlock due to a surge in requests that all acquire 1 permit and cannot acquire a second Unnecessary system throttling when a request acquires a single permit but can't acquire a second due to in flight requests and new requests that arrived after the first token acquisition but before the second. Half the number of permits available
1 parent 6b82523 commit af6b33f

File tree

2 files changed

+49
-45
lines changed

2 files changed

+49
-45
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
2+
title: Fix phaser/semaphore double registration bug in HttpJettySolrClient
3+
type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other
4+
authors:
5+
- name: James Vanneman
6+
links:
7+
- name: SOLR-18051
8+
url: https://issues.apache.org/jira/browse/SOLR-18051

solr/solrj/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java

Lines changed: 41 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -401,51 +401,47 @@ public CompletableFuture<NamedList<Object>> requestAsync(
401401
future.completeExceptionally(e);
402402
return future;
403403
}
404-
mrrv.request
405-
.onRequestQueued(asyncTracker.queuedListener)
406-
.onComplete(asyncTracker.completeListener)
407-
.send(
408-
new InputStreamResponseListener() {
409-
// MDC snapshot from requestAsync's thread
410-
MDCCopyHelper mdcCopyHelper = new MDCCopyHelper();
411-
412-
@Override
413-
public void onHeaders(Response response) {
414-
super.onHeaders(response);
415-
InputStreamResponseListener listener = this;
416-
executor.execute(
417-
() -> {
418-
InputStream is = listener.getInputStream();
419-
try {
420-
NamedList<Object> body =
421-
processErrorsAndResponse(solrRequest, response, is, url);
422-
mdcCopyHelper.onBegin(null);
423-
log.debug("response processing success");
424-
future.complete(body);
425-
} catch (CancellationException e) {
426-
mdcCopyHelper.onBegin(null);
427-
log.debug("response processing cancelled", e);
428-
if (!future.isDone()) {
429-
future.cancel(true);
430-
}
431-
} catch (Throwable e) {
432-
mdcCopyHelper.onBegin(null);
433-
log.debug("response processing failed", e);
434-
future.completeExceptionally(e);
435-
} finally {
436-
log.debug("response processing completed");
437-
mdcCopyHelper.onComplete(null);
438-
}
439-
});
440-
}
441-
442-
@Override
443-
public void onFailure(Response response, Throwable failure) {
444-
super.onFailure(response, failure);
445-
future.completeExceptionally(
446-
new SolrServerException(failure.getMessage(), failure));
447-
}
448-
});
404+
mrrv.request.send(
405+
new InputStreamResponseListener() {
406+
// MDC snapshot from requestAsync's thread
407+
MDCCopyHelper mdcCopyHelper = new MDCCopyHelper();
408+
409+
@Override
410+
public void onHeaders(Response response) {
411+
super.onHeaders(response);
412+
InputStreamResponseListener listener = this;
413+
executor.execute(
414+
() -> {
415+
InputStream is = listener.getInputStream();
416+
try {
417+
NamedList<Object> body =
418+
processErrorsAndResponse(solrRequest, response, is, url);
419+
mdcCopyHelper.onBegin(null);
420+
log.debug("response processing success");
421+
future.complete(body);
422+
} catch (CancellationException e) {
423+
mdcCopyHelper.onBegin(null);
424+
log.debug("response processing cancelled", e);
425+
if (!future.isDone()) {
426+
future.cancel(true);
427+
}
428+
} catch (Throwable e) {
429+
mdcCopyHelper.onBegin(null);
430+
log.debug("response processing failed", e);
431+
future.completeExceptionally(e);
432+
} finally {
433+
log.debug("response processing completed");
434+
mdcCopyHelper.onComplete(null);
435+
}
436+
});
437+
}
438+
439+
@Override
440+
public void onFailure(Response response, Throwable failure) {
441+
super.onFailure(response, failure);
442+
future.completeExceptionally(new SolrServerException(failure.getMessage(), failure));
443+
}
444+
});
449445

450446
// SOLR-17916: Disable request aborting
451447
// future.exceptionally(

0 commit comments

Comments
 (0)