Conversation
66b6815 to
fd6016a
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a thread-safe progress tracking mechanism for resumable uploads, consisting of the ResumableUploadProgressListener interface, the ResumableUploadStatus value class, and the UploadProgressTracker coordinator, along with comprehensive unit tests. Feedback on the changes suggests clearing the registered listeners list in UploadProgressTracker once a terminal state is reached to prevent potential memory leaks.
| @GuardedBy("lock") | ||
| private List<RegisteredListener> updateStatusLocked(ResumableUploadStatus newStatus) { | ||
| this.currentStatus = newStatus; | ||
| if (newStatus.getUploadUrl() != null && this.uploadSessionUrl == null) { | ||
| this.uploadSessionUrl = newStatus.getUploadUrl(); | ||
| } | ||
| return new ArrayList<>(this.listeners); | ||
| } |
There was a problem hiding this comment.
To prevent potential memory leaks and unnecessary resource retention, the list of registered listeners should be cleared once the tracker enters a terminal state (FINALIZED or FAILED). Since no further progress updates can occur after reaching a terminal state, holding onto the listeners (and potentially their enclosing classes or executors) is unnecessary.
@GuardedBy("lock")
private List<RegisteredListener> updateStatusLocked(ResumableUploadStatus newStatus) {
this.currentStatus = newStatus;
if (newStatus.getUploadUrl() != null && this.uploadSessionUrl == null) {
this.uploadSessionUrl = newStatus.getUploadUrl();
}
List<RegisteredListener> snapshot = new ArrayList<>(this.listeners);
if (newStatus.isTerminal()) {
this.listeners.clear();
}
return snapshot;
}fd6016a to
18db19d
Compare
18db19d to
b44760b
Compare
b44760b to
25bba5f
Compare
25bba5f to
bafa84d
Compare
bafa84d to
7b55d31
Compare
7b55d31 to
5f66ff4
Compare
|
|


Introduces
ResumableUploadStatus,UploadProgressListener, and thread-safeUploadProgressTracker. Tracks uploaded byte counts and state transitions across chunk attempts.