diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterPanel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterPanel.java
index 933189da12..5a4dfc4895 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterPanel.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterPanel.java
@@ -18,7 +18,10 @@ package ghidra.app.plugin.core.interpreter;
import java.awt.*;
import java.awt.event.*;
import java.io.*;
+import java.nio.charset.StandardCharsets;
import java.util.List;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.swing.*;
import javax.swing.text.*;
@@ -54,12 +57,12 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
private JScrollPane outputScrollPane;
private JTextPane outputTextPane;
private JTextPane promptTextPane;
- private JTextPane inputTextPane;
+ /* junit */ JTextPane inputTextPane;
private CodeCompletionWindow codeCompletionWindow;
private HistoryManager history;
- private IPStdin stdin;
+ /* junit */ IPStdin stdin;
private OutputStream stdout;
private OutputStream stderr;
private PrintWriter outWriter;
@@ -511,11 +514,10 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
private void repositionScrollpane() {
// NOTE: CRAZY CODE! subtract one to position short of final newline
- outputTextPane.setCaretPosition(outputTextPane.getDocument().getLength() - 1);
+ outputTextPane.setCaretPosition(Math.max(0, outputTextPane.getDocument().getLength() - 1));
}
void addText(String text, TextType type) {
- StyledDocument document = outputTextPane.getStyledDocument();
SimpleAttributeSet attributes;
switch (type) {
case STDERR:
@@ -530,6 +532,7 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
break;
}
try {
+ StyledDocument document = outputTextPane.getStyledDocument();
document.insertString(document.getLength(), text, attributes);
repositionScrollpane();
}
@@ -562,6 +565,7 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
public void clear() {
outputTextPane.setText("");
+ stdin.resetStream();
}
public String getOutputText() {
@@ -628,24 +632,7 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
public void dispose() {
- try {
- stdin.close();
- }
- catch (IOException e) {
- Msg.debug(this, "could not close stdin", e);
- }
-// try {
-// stdout.close();
-// }
-// catch (IOException e) {
-// Msg.warn(this, "could not close stdout", e);
-// }
-// try {
-// stderr.close();
-// }
-// catch (IOException e) {
-// Msg.warn(this, "could not close stderr", e);
-// }
+ stdin.close();
setVisible(false);
}
@@ -674,107 +661,105 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
// Inner Classes
//==================================================================================================
- private class IPStdin extends InputStream {
- private byte[] bytes;
+
+ /**
+ * An {@link InputStream} that has as its source text strings being pushed into
+ * it by a thread, and being read by another thread.
+ *
+ * Not thread-safe for multiple readers, but is thread-safe for writers.
+ *
+ * {@link #close() Closing} this stream (from any thread) will awaken the
+ * blocked reader thread and give an EOF result to the read operation it was blocking on.
+ */
+ /* junit vis */ static class IPStdin extends InputStream {
+ private static final byte[] EMPTY_BYTES = new byte[0];
+
+ // reader-thread only fields. write operations may not access/modify these
+ // fields.
+ private byte[] bytes = EMPTY_BYTES;
private int position = 0;
- private volatile boolean disposed;
+ // end reader-thread only fields
+
+ // shared reader / writer fields. Any thread may access these as they
+ // are threadsafe on their own
+ private LinkedBlockingQueue queuedBytes = new LinkedBlockingQueue<>();
+ private AtomicBoolean isClosed = new AtomicBoolean(false);
+ // end shared fields
+
+ private boolean fetchBytesFromQueue(boolean blocking) {
+
+ try {
+ // if the current byte buffer is exhausted, loop until we get
+ // a new non-empty byte buffer.
+ while (!isClosed.get() && position >= bytes.length) {
+ byte[] newBytes = blocking ? queuedBytes.take() : queuedBytes.poll();
+ if (newBytes == null) {
+ // this only happens when blocking == false, ie. a poll() operation
+ break;
+ }
+ bytes = newBytes;
+ position = 0;
+ }
+ }
+ catch (InterruptedException e) {
+ // fall thru to return which will return false
+ }
+
+ return position < bytes.length;
+ }
@Override
public int read(byte[] b, int off, int len) throws IOException {
- while (bytes == null) {
- try {
- synchronized (this) {
- this.wait();
- }
- }
- catch (InterruptedException e) {
- // handled below
- }
-
- if (disposed) {
- return -1;
- }
+ if (!fetchBytesFromQueue(true)) {
+ return -1;
}
- if (bytes != null) {
- int length = Math.min(bytes.length - position, len);
- System.arraycopy(bytes, position, b, off, length);
- if (position + length == bytes.length) {
- position = 0;
- bytes = null;
- }
- else {
- position += length;
- }
- return length;
- }
- return -1;
+ int length = Math.min(bytes.length - position, len);
+ System.arraycopy(bytes, position, b, off, length);
+ position += length;
+ return length;
}
@Override
public int read() throws IOException {
- while (bytes == null) {
- try {
- synchronized (this) {
- this.wait();
- }
- }
- catch (InterruptedException e) {
- // handled below
- }
-
- if (disposed) {
- return -1;
- }
-
+ byte[] buffer = new byte[1];
+ if (read(buffer, 0, 1) != 1) {
+ return -1;
}
-
- if (bytes != null) {
- int c = bytes[position] & 0xff;
- position++;
- if (position >= bytes.length) {
- position = 0;
- bytes = null;
- }
- return c;
- }
- return -1;
+ return buffer[0] & 0xff;
}
@Override
public int available() {
- if (bytes == null) {
- return 0;
+ fetchBytesFromQueue(false);
+ return bytes.length - position;
+ }
+
+ @Override
+ public void close() {
+ // this will wake up a blocked read-thread waiting on a read() operation
+ // and cause it to return a EOF result.
+ // All reads() after this close will return EOF value
+ isClosed.set(true);
+ queuedBytes.clear();
+ queuedBytes.offer(EMPTY_BYTES);
+ }
+
+ void addText(String text) {
+ if (!isClosed.get()) {
+ queuedBytes.offer(text.getBytes(StandardCharsets.UTF_8));
}
- return bytes.length;
}
/**
- * Overridden to stop this stream from blocking.
- *
- * @throws IOException not
+ * Resets this stream from a closed/always-eof state to an open state.
+ *
+ * Also clears any queued bytes. Safe to call even when open.
*/
- @Override
- public void close() throws IOException {
- disposed = true;
-
- synchronized (this) {
- notify(); // in case we are blocking
- }
- }
-
- synchronized void addText(String text) {
- if (bytes == null) {
- bytes = text.getBytes();
- position = 0;
- }
- else {
- byte[] temp = text.getBytes();
- byte[] newBytes = new byte[bytes.length + temp.length];
- System.arraycopy(bytes, 0, newBytes, 0, bytes.length);
- System.arraycopy(temp, 0, newBytes, bytes.length, temp.length);
- }
- this.notify();
+ void resetStream() {
+ isClosed.set(false);
+ queuedBytes.clear();
+ queuedBytes.offer(EMPTY_BYTES);
}
}
}
diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/interpreter/InterpreterPanelTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/interpreter/InterpreterPanelTest.java
new file mode 100644
index 0000000000..9adc528dca
--- /dev/null
+++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/interpreter/InterpreterPanelTest.java
@@ -0,0 +1,274 @@
+/* ###
+ * 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.app.plugin.core.interpreter;
+
+import static org.junit.Assert.*;
+
+import java.io.*;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.swing.*;
+import javax.swing.text.BadLocationException;
+import javax.swing.text.Document;
+
+import org.apache.commons.lang3.StringUtils;
+import org.junit.*;
+
+import ghidra.app.plugin.core.console.CodeCompletion;
+import ghidra.framework.options.ToolOptions;
+import ghidra.test.AbstractGhidraHeadedIntegrationTest;
+import ghidra.test.DummyTool;
+import resources.Icons;
+
+/**
+ * Test the InterpreterPanel/InterpreterConsole's stdIn InputStream handling by
+ * manually creating a JFrame (while a regular Ghidra tool is running)
+ * to host the panel in.
+ */
+public class InterpreterPanelTest extends AbstractGhidraHeadedIntegrationTest {
+
+ private JFrame frame;
+ private InterpreterPanel ip;
+ private JTextPane inputTextPane;
+ private Document inputDoc;
+ private BufferedReader reader;
+
+ @Before
+ public void setUp() throws Exception {
+
+ ip = createIP();
+ inputTextPane = ip.inputTextPane;
+ inputDoc = inputTextPane.getDocument();
+ reader = new BufferedReader(new InputStreamReader(ip.getStdin()));
+ frame = new JFrame("InterpreterPanel test frame");
+ frame.getContentPane().add(ip);
+ frame.setSize(400, 400);
+ runSwing(() -> frame.setVisible(true));
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ runSwing(() -> {
+ frame.setVisible(false);
+ frame.dispose();
+ });
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_AddRead() throws Exception {
+ doBackgroundTriggerTextTest(List.of("test1", "abc123"));
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_ClearResetsStream() throws Exception {
+ doBackgroundTriggerTextTest(List.of("test1", "abc123"));
+ doSwingMultilinePasteTest(List.of("testLine1", "testLine2", "testLine3"));
+
+ ip.clear();
+
+ doBackgroundTriggerTextTest(List.of("test2", "abc456"));
+ doSwingMultilinePasteTest(List.of("testLine4", "testLine5", "testLine6"));
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_CloseStreamBeforeReading() throws Exception {
+
+ ip.getStdin().close();
+
+ assertNull(reader.readLine()); // should always get NULL results because stream is now 'closed'
+ assertNull(reader.readLine()); // " "
+
+ ip.stdin.addText("test_while_closed\n"); // text added after close shouldn't be preserved
+ assertNull(reader.readLine()); // should always get NULL results because stream is now 'closed'
+ ip.clear(); // stream should now be open again
+ doBackgroundTriggerTextTest(List.of("test2", "abc456"));
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_CloseStreamWhileBlocking() throws Exception {
+
+ AtomicReference result = new AtomicReference<>("Non-null value");
+ doBackgroundReadLine(result); // this thread will block on readLine()
+
+ ip.getStdin().close();
+
+ // this will be set to null when readLine() returns
+ waitFor(() -> result.get() == null);
+
+ ip.clear(); // stream should now be open again
+ doBackgroundTriggerTextTest(List.of("test2", "abc456"));
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_InterruptStreamWhileBlocking() throws Exception {
+
+ AtomicReference result = new AtomicReference<>("Non-null value");
+ Thread t = doBackgroundReadLine(result); // this thread will block on readLine()
+
+ t.interrupt();
+
+ // this will be set to null when readLine() returns
+ waitFor(() -> result.get() == null);
+
+ ip.clear(); // stream should now be open again
+ doBackgroundTriggerTextTest(List.of("test2", "abc456"));
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_ReadSingleBytes() throws IOException {
+
+ doBackgroundPasteTest1AtATime("testvalue\n");
+
+ CountDownLatch startLatch = new CountDownLatch(1);
+ closeStreamViaBackgroundThread(startLatch);
+ startLatch.countDown();
+
+ assertEquals(-1, ip.getStdin().read());
+ }
+
+ @Test(timeout = 20000)
+ public void testInputStream_Available() throws IOException {
+
+ assertEquals(0, ip.getStdin().available());
+
+ triggerText(inputTextPane, "testvalue\n");
+ waitForSwing();
+
+ assertTrue(ip.getStdin().available() > 0);
+ }
+
+ private InterpreterPanel createIP() {
+ InterpreterConnection dummyIC = new InterpreterConnection() {
+ @Override
+ public String getTitle() {
+ return "Dummy Title";
+ }
+
+ @Override
+ public ImageIcon getIcon() {
+ return Icons.STOP_ICON;
+ }
+
+ @Override
+ public List getCompletions(String cmd) {
+ return List.of();
+ }
+ };
+
+ DummyTool tool = new DummyTool() {
+ @Override
+ public ToolOptions getOptions(String categoryName) {
+ return new ToolOptions("Dummy");
+ }
+ };
+ InterpreterPanel result = new InterpreterPanel(tool, dummyIC);
+ result.setPrompt("PROMPT:");
+ return result;
+ }
+
+ private void doSwingMultilinePasteTest(List multiLineTestValues)
+ throws IOException {
+ // simulates what happens during a paste when multi-line string with
+ // "\n"s is pasted.
+ runSwingLater(() -> {
+ try {
+ String multiLineString = StringUtils.join(multiLineTestValues, "\n") + '\n';
+ inputDoc.insertString(0, multiLineString, null);
+ }
+ catch (BadLocationException e) {
+ // ignore
+ }
+ });
+
+ for (String expectedValue : multiLineTestValues) {
+ String actualValue = reader.readLine();
+ assertEquals(expectedValue, actualValue);
+ }
+ }
+
+ private void doBackgroundPasteTest1AtATime(String testValue) throws IOException {
+ runSwingLater(() -> {
+ try {
+ inputDoc.insertString(0, testValue, null);
+ }
+ catch (BadLocationException e) {
+ // ignore
+ }
+ });
+ for (int i = 0; i < testValue.length(); i++) {
+ char expectedChar = testValue.charAt(i);
+ int actualValue = ip.getStdin().read();
+ assertEquals(expectedChar, (char) actualValue);
+ }
+ }
+
+ private void doBackgroundTriggerTextTest(List testValues) throws Exception {
+
+ new Thread(() -> {
+ for (String s : testValues) {
+ triggerText(inputTextPane, s + "\n");
+ }
+ }).start();
+
+ for (String expectedValue : testValues) {
+ String actualValue = reader.readLine();
+ assertEquals(expectedValue, actualValue);
+ }
+ }
+
+ private Thread doBackgroundReadLine(AtomicReference result)
+ throws InterruptedException {
+
+ CountDownLatch startLatch = new CountDownLatch(1);
+ Thread t = new Thread(() -> {
+ try {
+ startLatch.countDown();
+ result.set(reader.readLine());
+ }
+ catch (IOException e) {
+ // test will fail
+ }
+ });
+ t.start();
+
+ startLatch.await(2, TimeUnit.SECONDS); // the background thread is now spun-up and ready to read
+
+ // the smallest of sleeps to give the thread a chance to read after the latch was reached
+ sleep(10);
+
+ return t;
+ }
+
+ private void closeStreamViaBackgroundThread(CountDownLatch startLatch) {
+
+ new Thread(() -> {
+ try {
+ startLatch.await(2, TimeUnit.SECONDS);
+
+ // the smallest of sleeps to give the thread a chance to read after the latch was reached
+ sleep(10);
+
+ ip.getStdin().close();
+ }
+ catch (InterruptedException | IOException e) {
+ // ignore
+ }
+ }).start();
+ }
+}
diff --git a/Ghidra/Features/Python/src/main/java/ghidra/python/GhidraPythonInterpreter.java b/Ghidra/Features/Python/src/main/java/ghidra/python/GhidraPythonInterpreter.java
index 0f338e230c..d9261153f8 100644
--- a/Ghidra/Features/Python/src/main/java/ghidra/python/GhidraPythonInterpreter.java
+++ b/Ghidra/Features/Python/src/main/java/ghidra/python/GhidraPythonInterpreter.java
@@ -245,8 +245,14 @@ public class GhidraPythonInterpreter extends InteractiveInterpreter {
* @param str The string to print.
*/
void printErr(String str) {
- getSystemState().stderr.invoke("write", new PyString(str + "\n"));
- getSystemState().stderr.invoke("flush");
+ try {
+ getSystemState().stderr.invoke("write", new PyString(str + "\n"));
+ getSystemState().stderr.invoke("flush");
+ }
+ catch (PyException e) {
+ // if the python interp state's stdin/stdout/stderr is messed up, it can throw an error
+ Msg.error(this, "Failed to write to stderr", e);
+ }
}
/**
diff --git a/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPlugin.java b/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPlugin.java
index f2c24b4e05..667ce8f270 100644
--- a/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPlugin.java
+++ b/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPlugin.java
@@ -59,6 +59,8 @@ import resources.ResourceManager;
public class PythonPlugin extends ProgramPlugin
implements InterpreterConnection, OptionsChangeListener {
+ private final static int INPUT_THREAD_SHUTDOWN_TIMEOUT_MS = 1000;
+
private InterpreterConsole console;
private GhidraPythonInterpreter interpreter;
private PythonScript interactiveScript;
@@ -202,7 +204,8 @@ public class PythonPlugin extends ProgramPlugin
PythonCodeCompletionFactory.setupOptions(this, options);
}
else {
- inputThread.dispose();
+ inputThread.shutdown();
+ inputThread = null;
interpreter.cleanup();
interpreter = GhidraPythonInterpreter.get();
}
@@ -278,7 +281,7 @@ public class PythonPlugin extends ProgramPlugin
// Terminate the input thread
if (inputThread != null) {
- inputThread.dispose();
+ inputThread.shutdown();
inputThread = null;
}
diff --git a/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPluginInputThread.java b/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPluginInputThread.java
index 90c7dee1e7..3057e2aae9 100644
--- a/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPluginInputThread.java
+++ b/Ghidra/Features/Python/src/main/java/ghidra/python/PythonPluginInputThread.java
@@ -28,10 +28,11 @@ class PythonPluginInputThread extends Thread {
private static int generationCount = 0;
- private PythonPlugin plugin;
+ private final PythonPlugin plugin;
+ private final AtomicBoolean moreInputWanted = new AtomicBoolean(false);
+ private final AtomicBoolean shutdownRequested = new AtomicBoolean(false);
+ private final InputStream consoleStdin;
private PythonPluginExecutionThread pythonExecutionThread;
- private AtomicBoolean moreInputWanted;
- private AtomicBoolean shouldContinue;
/**
* Creates a new python input thread that gets a line of python input from the given plugin.
@@ -41,8 +42,7 @@ class PythonPluginInputThread extends Thread {
PythonPluginInputThread(PythonPlugin plugin) {
super("Python plugin input thread (generation " + ++generationCount + ")");
this.plugin = plugin;
- this.moreInputWanted = new AtomicBoolean(false);
- this.shouldContinue = new AtomicBoolean(true);
+ this.consoleStdin = plugin.getConsole().getStdin();
}
/**
@@ -56,28 +56,13 @@ class PythonPluginInputThread extends Thread {
@Override
public void run() {
- try (BufferedReader reader =
- new BufferedReader(new InputStreamReader(plugin.getConsole().getStdin()))) {
- while (shouldContinue.get()) {
-
- // Read a line from the console. Do it non-blocking so we can exit the thread
- // if we were instructed to not continue.
- String line;
- if (plugin.getConsole().getStdin().available() > 0) {
- line = reader.readLine();
- }
- else {
- try {
- Thread.sleep(50);
- }
- catch (InterruptedException e) {
- // Nothing to do...just continue.
- }
- continue;
- }
+ try (BufferedReader reader = new BufferedReader(new InputStreamReader(consoleStdin))) {
+ String line;
+ while (!shutdownRequested.get() && (line = reader.readLine()) != null) {
// Execute the line in a new thread
- pythonExecutionThread = new PythonPluginExecutionThread(plugin, line, moreInputWanted);
+ pythonExecutionThread =
+ new PythonPluginExecutionThread(plugin, line, moreInputWanted);
pythonExecutionThread.start();
try {
@@ -90,9 +75,10 @@ class PythonPluginInputThread extends Thread {
}
// Set the prompt appropriately
- plugin.getConsole().setPrompt(
- moreInputWanted.get() ? plugin.getInterpreter().getSecondaryPrompt()
- : plugin.getInterpreter().getPrimaryPrompt());
+ plugin.getConsole()
+ .setPrompt(
+ moreInputWanted.get() ? plugin.getInterpreter().getSecondaryPrompt()
+ : plugin.getInterpreter().getPrimaryPrompt());
}
}
catch (IOException e) {
@@ -103,9 +89,17 @@ class PythonPluginInputThread extends Thread {
}
/**
- * Disposes of the thread by telling it to not continue asking for input.
+ * Causes the the background thread's run() loop to exit.
+ *
+ * Causes background thread's exit by closing the inputstream it is looping on.
*/
- void dispose() {
- shouldContinue.set(false);
+ void shutdown() {
+ try {
+ shutdownRequested.set(true);
+ consoleStdin.close();
+ }
+ catch (IOException e) {
+ // shouldn't happen, ignore
+ }
}
}