From 793bedc0b436cb5c8adce7445f993fdaa391723b Mon Sep 17 00:00:00 2001 From: dev747368 <48332326+dev747368@users.noreply.github.com> Date: Tue, 20 Jun 2023 22:10:00 +0000 Subject: [PATCH] GP-3554 fix UnknownProgressWrappingTaskMonitor's checkCanceled The 1L checkCanceled() was calling the base-class's impl instead of calling the delegate's impl. Fixed by use the right base class. Also tweaked 2 new uses of 1L in Dtb and Fdt Analyzer. --- .../ghidra/file/formats/dtb/DtbAnalyzer.java | 2 +- .../ghidra/file/formats/dtb/FdtAnalyzer.java | 2 +- .../ghidra/util/task/TaskMonitorAdapter.java | 11 ++++ .../ghidra/util/task/WrappingTaskMonitor.java | 6 +++ .../UnknownProgressWrappingTaskMonitor.java | 27 ++-------- ...nknownProgressWrappingTaskMonitorTest.java | 51 +++++++++++++++++++ 6 files changed, 74 insertions(+), 25 deletions(-) create mode 100644 Ghidra/Framework/Gui/src/test/java/ghidra/util/task/UnknownProgressWrappingTaskMonitorTest.java diff --git a/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/DtbAnalyzer.java b/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/DtbAnalyzer.java index 53cd95e651..4c387ac684 100644 --- a/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/DtbAnalyzer.java +++ b/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/DtbAnalyzer.java @@ -89,7 +89,7 @@ public class DtbAnalyzer extends FileFormatAnalyzer { monitor.initialize(header.getEntries().size()); for (int i = 0; i < header.getEntries().size(); ++i) { - monitor.checkCanceled(); + monitor.checkCancelled(); monitor.incrementProgress(1); DtTableEntry entry = header.getEntries().get(i); diff --git a/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/FdtAnalyzer.java b/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/FdtAnalyzer.java index 869d204cbf..483b29ab6a 100644 --- a/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/FdtAnalyzer.java +++ b/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/dtb/FdtAnalyzer.java @@ -79,7 +79,7 @@ public class FdtAnalyzer extends FileFormatAnalyzer { Address address = program.getMinAddress(); while (true) { - monitor.checkCanceled(); + monitor.checkCancelled(); if (address.compareTo(program.getMaxAddress()) >= 0) { break; diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/TaskMonitorAdapter.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/TaskMonitorAdapter.java index 03ceeb3d29..5c0e23a1d5 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/TaskMonitorAdapter.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/TaskMonitorAdapter.java @@ -24,6 +24,9 @@ import ghidra.util.exception.CancelledException; *

* This class supports cancelling and cancel listener notification. Cancelling must be enabled * via {@link #setCancelEnabled(boolean)}. + *

+ * Use {@link WrappingTaskMonitor} if you need to override an existing TaskMonitor + * instance's behavior. */ public class TaskMonitorAdapter implements TaskMonitor { @@ -54,6 +57,7 @@ public class TaskMonitorAdapter implements TaskMonitor { return cancelled; } + @Deprecated(since = "10.3") @Override public void checkCanceled() throws CancelledException { if (cancelled) { @@ -61,6 +65,13 @@ public class TaskMonitorAdapter implements TaskMonitor { } } + @Override + public void checkCancelled() throws CancelledException { + if (cancelled) { + throw new CancelledException(); + } + } + @Override public void setMessage(String message) { // do nothing diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/WrappingTaskMonitor.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/WrappingTaskMonitor.java index b6289b8401..7c1c30ca0e 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/WrappingTaskMonitor.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/WrappingTaskMonitor.java @@ -133,11 +133,17 @@ public class WrappingTaskMonitor implements TaskMonitor { return delegate.isIndeterminate(); } + @Deprecated(since = "10.3") @Override public void checkCanceled() throws CancelledException { delegate.checkCancelled(); } + @Override + public void checkCancelled() throws CancelledException { + delegate.checkCancelled(); + } + @Override public void incrementProgress(long incrementAmount) { delegate.incrementProgress(incrementAmount); diff --git a/Ghidra/Framework/Gui/src/main/java/ghidra/util/task/UnknownProgressWrappingTaskMonitor.java b/Ghidra/Framework/Gui/src/main/java/ghidra/util/task/UnknownProgressWrappingTaskMonitor.java index 2a0de6fa3c..e25a9c16d3 100644 --- a/Ghidra/Framework/Gui/src/main/java/ghidra/util/task/UnknownProgressWrappingTaskMonitor.java +++ b/Ghidra/Framework/Gui/src/main/java/ghidra/util/task/UnknownProgressWrappingTaskMonitor.java @@ -15,48 +15,29 @@ */ package ghidra.util.task; -import ghidra.util.exception.CancelledException; - /** * A class that is meant to wrap a {@link TaskMonitor} when you do not know the maximum value * of the progress. */ -public class UnknownProgressWrappingTaskMonitor extends TaskMonitorAdapter { - - private TaskMonitor delegate; +public class UnknownProgressWrappingTaskMonitor extends WrappingTaskMonitor { public UnknownProgressWrappingTaskMonitor(TaskMonitor delegate, long startMaximum) { - this.delegate = delegate; + super(delegate); delegate.setMaximum(startMaximum); } - @Override - public void setMessage(String message) { - delegate.setMessage(message); - } - @Override public void setProgress(long value) { - delegate.setProgress(value); + super.setProgress(value); maybeUpdateMaximum(); } @Override public void incrementProgress(long incrementAmount) { - delegate.incrementProgress(incrementAmount); + super.incrementProgress(incrementAmount); maybeUpdateMaximum(); } - @Override - public synchronized boolean isCancelled() { - return delegate.isCancelled(); - } - - @Override - public void checkCancelled() throws CancelledException { - delegate.checkCancelled(); - } - private void maybeUpdateMaximum() { long currentMaximum = delegate.getMaximum(); long progress = delegate.getProgress(); diff --git a/Ghidra/Framework/Gui/src/test/java/ghidra/util/task/UnknownProgressWrappingTaskMonitorTest.java b/Ghidra/Framework/Gui/src/test/java/ghidra/util/task/UnknownProgressWrappingTaskMonitorTest.java new file mode 100644 index 0000000000..ecb8c564cd --- /dev/null +++ b/Ghidra/Framework/Gui/src/test/java/ghidra/util/task/UnknownProgressWrappingTaskMonitorTest.java @@ -0,0 +1,51 @@ +/* ### + * IP: GHIDRA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ghidra.util.task; + +import static org.junit.Assert.*; + +import org.junit.Test; + +import generic.test.AbstractGenericTest; +import ghidra.util.exception.CancelledException; + +public class UnknownProgressWrappingTaskMonitorTest extends AbstractGenericTest { + + @Test + public void testUPWTM_checkCanceled_1L_vs_2L() { + TaskMonitorAdapter monitor = new TaskMonitorAdapter(true); + monitor.cancel(); + UnknownProgressWrappingTaskMonitor upwtm = + new UnknownProgressWrappingTaskMonitor(monitor, 100); + try { + upwtm.checkCanceled(); + fail(); + } + catch (CancelledException e) { + // good + } + + try { + upwtm.checkCancelled(); + fail(); + } + catch (CancelledException e) { + // good + } + + } + +}