janhoy commented on code in PR #4236:
URL: https://github.com/apache/solr/pull/4236#discussion_r2987175207
##########
solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java:
##########
@@ -844,28 +870,48 @@ private static class AsyncTracker {
private final Response.CompleteListener completeListener;
AsyncTracker() {
+ maxRequests = Integer.getInteger(ASYNC_REQUESTS_MAX_SYSPROP,
MAX_OUTSTANDING_REQUESTS);
// TODO: what about shared instances?
phaser = new Phaser(1);
- available = new Semaphore(MAX_OUTSTANDING_REQUESTS, false);
+ available = new Semaphore(maxRequests, false);
queuedListener =
request -> {
+ if (request.getAttributes().get(PERMIT_ACQUIRED_ATTR) != null) {
+ return;
+ }
phaser.register();
try {
available.acquire();
- } catch (InterruptedException ignored) {
-
+ } catch (InterruptedException e) {
+ // Undo phaser registration: no permit was acquired so
completeListener must not
+ // release.
+ phaser.arriveAndDeregister();
+ Thread.currentThread().interrupt();
+ return;
}
+ request.attribute(PERMIT_ACQUIRED_ATTR, Boolean.TRUE);
};
completeListener =
result -> {
- phaser.arriveAndDeregister();
- available.release();
+ if (result == null
Review Comment:
Sometimes result is `null` here, so we can't fetch request attribute.
I wonder if this change here is not needed. The attribute's mission is to
avoid double acquire of permit, but is there also a potential double
`onComplete()` call to guard against?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
