Skip to content

Commit

Permalink
Fix potential memory leak in UI
Browse files Browse the repository at this point in the history
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
  • Loading branch information
coeuvre authored and traversaro committed Jun 24, 2023
1 parent 0bd3344 commit ffa2073
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,12 @@ public ListenableFuture<Void> downloadFile(
() -> {
try {
out.close();
reporter.finished();
} catch (IOException e) {
logger.atWarning().withCause(e).log(
"Unexpected exception closing output stream after downloading %s/%d to %s",
digest.getHash(), digest.getSizeBytes(), path);
} finally {
reporter.finished();
}
},
directExecutor());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,11 @@ public void afterCommand(AfterCommandEvent event) {
public void downloadProgress(FetchProgress event) {
maybeAddDate();
stateTracker.downloadProgress(event);
refresh();
if (!event.isFinished()) {
refresh();
} else {
checkActivities();
}
}

@Subscribe
Expand Down

0 comments on commit ffa2073

Please sign in to comment.