Skip to content

Commit 59748fb

Browse files
coeuvreiancha1992
authored andcommitted
Fix potential memory leak in UI
We report download progress to UI when downloading outputs from remote cache. UI thread keeps track of active downloads. There are two cases the UI thread could leak memory: 1. If we failed to close the output stream, the `reporter.finished()` will never be called, prevent UI thread from releasing the active download. This is fixed by calling `reporter.finished()` in `finally` block. 2. Normally, UI thread stops after `BuildCompleted` event. However, if we have background download after build is completed, UI thread is not stopped to continue printing out download progress. But after all downloads are done, we forgot to stop the UI thread, resulting all referenced objects leaked. This is fixed by calling `checkActivities()` for every download progress. Fixes bazelbuild#18145. Closes bazelbuild#18593. PiperOrigin-RevId: 539923685 Change-Id: I7e2887035e540b39e382ab5fcbc06bad03b10427
1 parent ea4ad30 commit 59748fb

File tree

2 files changed

+7
-2
lines changed

2 files changed

+7
-2
lines changed

src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,11 +379,12 @@ public ListenableFuture<Void> downloadFile(
379379
() -> {
380380
try {
381381
out.close();
382-
reporter.finished();
383382
} catch (IOException e) {
384383
logger.atWarning().withCause(e).log(
385384
"Unexpected exception closing output stream after downloading %s/%d to %s",
386385
digest.getHash(), digest.getSizeBytes(), path);
386+
} finally {
387+
reporter.finished();
387388
}
388389
},
389390
directExecutor());

src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,11 @@ public void afterCommand(AfterCommandEvent event) {
670670
public void downloadProgress(FetchProgress event) {
671671
maybeAddDate();
672672
stateTracker.downloadProgress(event);
673-
refresh();
673+
if (!event.isFinished()) {
674+
refresh();
675+
} else {
676+
checkActivities();
677+
}
674678
}
675679

676680
@Subscribe

0 commit comments

Comments
 (0)