Ensure added Closeables closed even if viewmodel cleared
Although developers should not be adding any logic after
viewmodel.onCleared() has been called, we should ensure
that Closeables are still closed if they are attempted
to be added to the ViewModel to prevent potential leaks.
RelNote: "`ViewModel`'s `addCloseable()` now checks whether
the view model has already been cleared before adding the
closeable to prevent leaks."
Test: testAddCloseableAlreadyClearedVM
Fixes: 280294730
Change-Id: I4712ee3600acbf04506af46e6ac479a617cebd86
diff --git a/lifecycle/lifecycle-viewmodel/src/main/java/androidx/lifecycle/ViewModel.java b/lifecycle/lifecycle-viewmodel/src/main/java/androidx/lifecycle/ViewModel.java
index 2370b740..6be73f6 100644
--- a/lifecycle/lifecycle-viewmodel/src/main/java/androidx/lifecycle/ViewModel.java
+++ b/lifecycle/lifecycle-viewmodel/src/main/java/androidx/lifecycle/ViewModel.java
@@ -139,11 +139,22 @@
/**
* Add a new {@link Closeable} object that will be closed directly before
* {@link #onCleared()} is called.
- *
+ * <p>
+ * If onCleared() has already been called, the closeable will not be added,
+ * and will instead be closed immediately.
+ * <p>
* @param closeable The object that should be {@link Closeable#close() closed} directly before
* {@link #onCleared()} is called.
*/
public void addCloseable(@NonNull Closeable closeable) {
+ // Although no logic should be done after user calls onCleared(), we will
+ // ensure that if it has already been called, the closeable attempting to
+ // be added will be closed immediately to ensure there will be no leaks.
+ if (mCleared) {
+ closeWithRuntimeException(closeable);
+ return;
+ }
+
// As this method is final, it will still be called on mock objects even
// though mCloseables won't actually be created...we'll just not do anything
// in that case.
@@ -186,6 +197,7 @@
closeWithRuntimeException(closeable);
}
}
+ mCloseables.clear();
}
onCleared();
}
diff --git a/lifecycle/lifecycle-viewmodel/src/test/java/androidx/lifecycle/ViewModelTest.kt b/lifecycle/lifecycle-viewmodel/src/test/java/androidx/lifecycle/ViewModelTest.kt
index 6e68344..5e97e09 100644
--- a/lifecycle/lifecycle-viewmodel/src/test/java/androidx/lifecycle/ViewModelTest.kt
+++ b/lifecycle/lifecycle-viewmodel/src/test/java/androidx/lifecycle/ViewModelTest.kt
@@ -80,6 +80,16 @@
}
@Test
+ fun testAddCloseableAlreadyClearedVM() {
+ val vm = ViewModel()
+ vm.clear()
+ val impl = CloseableImpl()
+ // This shouldn't crash, even though vm already cleared
+ vm.addCloseable(impl)
+ assertTrue(impl.wasClosed)
+ }
+
+ @Test
fun testConstructorCloseable() {
val impl = CloseableImpl()
val vm = ConstructorArgViewModel(impl)
