Skip to content

Commit 5e352af

Browse files
larsrc-googlecopybara-github
authored andcommitted
Fix bug in WorkRequestHandler's handling of singleplex requests that would cause occasional hangs.
RELNOTES: None. PiperOrigin-RevId: 386194200
1 parent 951a302 commit 5e352af

File tree

2 files changed

+32
-11
lines changed

2 files changed

+32
-11
lines changed

src/main/java/com/google/devtools/build/lib/worker/WorkRequestHandler.java

+24-10
Original file line numberDiff line numberDiff line change
@@ -305,26 +305,40 @@ public WorkRequestHandler build() {
305305
* returns. If {@code in} reaches EOF, it also returns.
306306
*/
307307
public void processRequests() throws IOException {
308-
while (true) {
309-
WorkRequest request = messageProcessor.readWorkRequest();
310-
if (request == null) {
311-
break;
312-
}
313-
if (request.getCancel()) {
314-
respondToCancelRequest(request);
315-
} else {
316-
startResponseThread(request);
308+
try {
309+
while (true) {
310+
WorkRequest request = messageProcessor.readWorkRequest();
311+
if (request == null) {
312+
break;
313+
}
314+
if (request.getCancel()) {
315+
respondToCancelRequest(request);
316+
} else {
317+
startResponseThread(request);
318+
}
317319
}
320+
} catch (InterruptedException e) {
321+
Thread.currentThread().interrupt();
322+
stderr.println("InterruptedException processing requests.");
318323
}
319324
}
320325

321326
/** Starts a thread for the given request. */
322-
void startResponseThread(WorkRequest request) {
327+
void startResponseThread(WorkRequest request) throws InterruptedException {
323328
Thread currentThread = Thread.currentThread();
324329
String threadName =
325330
request.getRequestId() > 0
326331
? "multiplex-request-" + request.getRequestId()
327332
: "singleplex-request";
333+
// TODO(larsrc): See if this can be handled with a queue instead, without introducing more
334+
// race conditions.
335+
if (request.getRequestId() == 0) {
336+
while (activeRequests.containsKey(request.getRequestId())) {
337+
// b/194051480: Previous singleplex requests can still be in activeRequests for a bit after
338+
// the response has been sent. We need to wait for them to vanish.
339+
Thread.sleep(1);
340+
}
341+
}
328342
Thread t =
329343
new Thread(
330344
() -> {

src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,14 @@ public void processRequests() throws IOException {
9898
if (request.getCancel()) {
9999
respondToCancelRequest(request);
100100
} else {
101-
startResponseThread(request);
101+
try {
102+
startResponseThread(request);
103+
} catch (InterruptedException e) {
104+
// We don't expect interrupts at this level, only inside the individual request
105+
// handling threads, so here we just abort on interrupt.
106+
e.printStackTrace();
107+
return;
108+
}
102109
}
103110
if (workerOptions.exitAfter > 0 && workUnitCounter > workerOptions.exitAfter) {
104111
System.exit(0);

0 commit comments

Comments
 (0)