diff --git a/src/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterController.java b/src/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterController.java index 4053f71e..d1f6e884 100644 --- a/src/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterController.java +++ b/src/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterController.java @@ -28,6 +28,7 @@ import org.bitbucket.mlopatkin.android.logviewer.ui.indexfilter.IndexFilterCollection; import org.bitbucket.mlopatkin.android.logviewer.ui.mainframe.MainFrameScoped; import org.bitbucket.mlopatkin.android.logviewer.ui.mainframe.popupmenu.MenuFilterCreator; +import org.bitbucket.mlopatkin.utils.Threads; import java.util.ArrayList; import java.util.List; @@ -88,7 +89,9 @@ public void setBufferEnabled(LogRecord.Buffer buffer, boolean enabled) { @Override public void createFilterWithDialog() { - dialogFactory.startCreateFilterDialog().thenAccept(newFilter -> newFilter.ifPresent(this::addNewDialogFilter)); + dialogFactory.startCreateFilterDialog() + .thenAccept(newFilter -> newFilter.ifPresent(this::addNewDialogFilter)) + .exceptionally(Threads::uncaughtException); } private DialogPanelFilter createDialogPanelFilter(FilterFromDialog filter) { @@ -132,7 +135,9 @@ public void addFilter(FilterFromDialog filter) { @Override public void createFilterWithDialog(FilterFromDialog baseData) { - dialogFactory.startEditFilterDialog(baseData).thenAccept(result -> result.ifPresent(this::addFilter)); + dialogFactory.startEditFilterDialog(baseData) + .thenAccept(result -> result.ifPresent(this::addFilter)) + .exceptionally(Threads::uncaughtException); } /** @@ -175,7 +180,15 @@ public void delete() { protected void replaceMeWith(BaseToggleFilter replacement) { replacement.isEnabled = isEnabled; int myPos = filters.indexOf(this); - assert myPos >= 0; + if (myPos == -1) { + // Ignore edit result if |this| filter isn't alive anymore. This can happen if the editor was opened + // twice for the same filter. If both edits complete successfully then second one won't find 'this' + // here, because it was replaced with the result of the first edit. Not so much can be done in this + // case. Prior to 0.19 the result of the second edit was added as a new filter, 0.19 just crashes. + // TODO(mlopatkin) proper solution is to disallow opening a second editor but this is much more involved + // fix which I don't like to push within 0.20 timeframe. + return; + } filters.set(myPos, replacement); filterPanelModel.replaceFilter(this, replacement); if (collection.equals(replacement.collection) && mode == replacement.mode) { @@ -209,7 +222,7 @@ public void openFilterEditor() { DialogPanelFilter newPanelFilter = createDialogPanelFilter(newFilter.get()); replaceMeWith(newPanelFilter); } - }); + }).exceptionally(Threads::uncaughtException); } @Override diff --git a/test/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterControllerTest.java b/test/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterControllerTest.java index 5a08f4af..474c9e23 100644 --- a/test/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterControllerTest.java +++ b/test/org/bitbucket/mlopatkin/android/logviewer/filters/MainFilterControllerTest.java @@ -160,6 +160,25 @@ public void testSavingAndInitializngFromSaved() throws Exception { assertEquals(Color.BLACK, filterImpl.getHighlightColor(RECORD2)); } + @Test + public void editFilterTwiceDoesntCrash() { + MainFilterController controller = new MainFilterController( + filterPanelModel, indexFilterCollection, dialogFactory, new FakeDefaultConfigStorage(), filterImpl); + + order = inOrder(dialogFactory, filterPanelModel); + FilterFromDialog initialFilter = createColoringFilter(Color.BLACK, MATCH_ALL); + + PanelFilter initialPanel = createFilterWithDialog(controller, initialFilter); + + CompletableFuture> firstDialog = openFilterDialog(initialPanel); + CompletableFuture> secondDialog = openFilterDialog(initialPanel); + + firstDialog.complete(Optional.of(createColoringFilter(Color.BLUE, MATCH_ALL))); + secondDialog.complete(Optional.of(createColoringFilter(Color.RED, MATCH_ALL))); + + assertEquals(Color.BLUE, filterImpl.getHighlightColor(RECORD1)); + } + private PanelFilter createFilterWithDialog(MainFilterController controller, FilterFromDialog dialogResult) { when(dialogFactory.startCreateFilterDialog()).thenReturn( CompletableFuture.completedFuture(Optional.of(dialogResult))); @@ -178,6 +197,14 @@ private PanelFilter editFilterWithDialog(FilterFromDialog newFilter, PanelFilter return panelFilterCaptor.getValue(); } + private CompletableFuture> openFilterDialog(PanelFilter filter) { + CompletableFuture> future = new CompletableFuture<>(); + when(dialogFactory.startEditFilterDialog(any())).thenReturn(future); + filter.openFilterEditor(); + + return future; + } + private FilterFromDialog createMockFilter(FilteringMode mode, final Predicate predicate) { FilterFromDialog result = new FilterFromDialog() { @Override diff --git a/utils/org/bitbucket/mlopatkin/utils/Threads.java b/utils/org/bitbucket/mlopatkin/utils/Threads.java index 1b3f4a3f..5e41c614 100644 --- a/utils/org/bitbucket/mlopatkin/utils/Threads.java +++ b/utils/org/bitbucket/mlopatkin/utils/Threads.java @@ -16,7 +16,11 @@ package org.bitbucket.mlopatkin.utils; +import org.checkerframework.checker.nullness.qual.Nullable; + +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ThreadFactory; +import java.util.function.Function; public final class Threads { private Threads() {} @@ -30,4 +34,16 @@ private Threads() {} public static ThreadFactory withName(final String name) { return r -> new Thread(r, name); } + + /** + * Helper for {@link CompletableFuture#exceptionally(Function)} that forwards exception to thread's default + * exception handler. + * @param th the throwable to forward + * @return {@code null} + */ + public static @Nullable Void uncaughtException(Throwable th) { + Thread thread = Thread.currentThread(); + thread.getUncaughtExceptionHandler().uncaughtException(thread, th); + return null; + } }