diff --git a/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/gtk/org/eclipse/swt/dnd/ClipboardProxy.java b/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/gtk/org/eclipse/swt/dnd/ClipboardProxy.java index 92068b23c4..b89dd0282d 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/gtk/org/eclipse/swt/dnd/ClipboardProxy.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/gtk/org/eclipse/swt/dnd/ClipboardProxy.java @@ -34,6 +34,9 @@ class ClipboardProxy { long clipboardOwner = GTK3.gtk_window_new(GTK.GTK_WINDOW_TOPLEVEL); + /** + * display == null means that the display has been disposed. + */ Display display; Clipboard activeClipboard = null; Clipboard activePrimaryClipboard = null; @@ -78,7 +81,48 @@ void gtk_gdk_clipboard_clear(long clipboard) { GTK3.gtk_clipboard_clear(clipboard); } +/** + * The calls to gtk_clipboard_set_with_owner (in + * {@link #setData(Clipboard, Object[], Transfer[], int)}) means the + * GtkClipboard (in C side) has function pointers saved to the getFunc + + * clearFunc callbacks. + * + * Therefore, when we dispose {@link ClipboardProxy} we cannot dispose the + * callbacks until we know that GtkClipboard doesn't have a pointer to these + * callbacks. GtkClipboard clears these pointers in clipboard_unset + * (https://gitlab.gnome.org/GNOME/gtk/-/blob/716458e86a222f43e64f7a4feda37749f3469ee4/gtk/gtkclipboard.c#L755) + * and notifies us that the pointers are no longer stored by calling clearFunc + * (https://gitlab.gnome.org/GNOME/gtk/-/blob/716458e86a222f43e64f7a4feda37749f3469ee4/gtk/gtkclipboard.c#L782) + * + * Therefore, after disposing ClipboardProxy we need to defer disposing the + * callbacks until clearFunc has been called. + * + * We know if we have been called sufficiently (for both clipboards that could + * have a handle stored) when both clipboards no longer have data stored. + * + * If we don't defer the disposal it causes SIGSEGV or other undefined behavior. + * + * Fix for https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + */ +private void finishDispose() { + if (display == null) { + if (clipboardDataTypes == null && primaryClipboardDataTypes == null) { + if (getFunc != null ) getFunc.dispose(); + getFunc = null; + if (clearFunc != null) clearFunc.dispose(); + clearFunc = null; + if (clipboardOwner != 0) { + GTK3.gtk_widget_destroy(clipboardOwner); + } + clipboardOwner = 0; + } + } +} + long clearFunc(long clipboard,long user_data_or_owner){ + if (display == null) { + System.err.println("***WARNING: Attempt to access SWT clipboard after disposing SWT Display."); + } if (clipboard == Clipboard.GTKCLIPBOARD) { activeClipboard = null; clipboardData = null; @@ -89,6 +133,7 @@ long clearFunc(long clipboard,long user_data_or_owner){ primaryClipboardData = null; primaryClipboardDataTypes = null; } + finishDispose(); return 1; } @@ -101,18 +146,7 @@ void dispose () { GTK3.gtk_clipboard_store(Clipboard.GTKPRIMARYCLIPBOARD); } display = null; - if (getFunc != null ) getFunc.dispose(); - getFunc = null; - if (clearFunc != null) clearFunc.dispose(); - clearFunc = null; - clipboardData = null; - clipboardDataTypes = null; - primaryClipboardData = null; - primaryClipboardDataTypes = null; - if (clipboardOwner != 0) { - GTK3.gtk_widget_destroy(clipboardOwner); - } - clipboardOwner = 0; + finishDispose(); } /** @@ -120,6 +154,10 @@ void dispose () { * When this clipboard is disposed, the data will no longer be available. */ long getFunc(long clipboard, long selection_data, long info, long user_data_or_owner){ + if (display == null) { + System.err.println("***WARNING: Attempt to access SWT clipboard after disposing SWT Display."); + return 0; + } if (selection_data == 0) return 0; long target = GTK3.gtk_selection_data_get_target(selection_data); TransferData tdata = new TransferData(); diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java index 711d63394f..1f1a4ae4b4 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java @@ -149,6 +149,28 @@ public final static boolean isWayland() { throw new IllegalStateException("unreachable"); } + /** + * Return whether running on GTK3. This is dynamically set at runtime and cannot + * be accessed before the corresponding property is initialized in OS. + * + * Note: this method still must be called after the static + * block of OS is run. + */ + public final static boolean isGTK3() { + if (!isGTK) { + return false; + } + + String version = System.getProperty("org.eclipse.swt.internal.gtk.version", ""); + if (version.startsWith("3")) { + return true; + } else if (!version.isBlank()) { + return false; + } + fail("org.eclipse.swt.internal.gtk.version System property is not set yet. Create a new Display (or otherwise access OS) before calling isGTK4"); + throw new IllegalStateException("unreachable"); + } + /** * Return whether running on GTK4. This is dynamically set at runtime and cannot * be accessed before the corresponding property is initialized in OS. diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_dnd_Clipboard.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_dnd_Clipboard.java index 04ef387f37..ebf030a717 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_dnd_Clipboard.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_dnd_Clipboard.java @@ -19,11 +19,14 @@ import java.util.Arrays; import java.util.concurrent.CompletableFuture; +import org.eclipse.swt.SWT; import org.eclipse.swt.dnd.DND; import org.eclipse.swt.dnd.RTFTransfer; import org.eclipse.swt.dnd.TextTransfer; import org.eclipse.swt.dnd.Transfer; import org.eclipse.swt.dnd.TransferData; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Text; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.MethodOrderer.OrderAnnotation; import org.junit.jupiter.api.Order; @@ -243,4 +246,137 @@ public void test_getAvailableTypes(int clipboardId) throws Exception { availableTypes = clipboard.getAvailableTypes(clipboardId); assertTrue(Arrays.stream(availableTypes).anyMatch(rtfTransfer::isSupportedType)); } + + /** + * tear down and start again without clearing off the clipboard + */ + private void tearDownAndStartAgain() { + // we can't call tearDown because we don't want to clear the clipboard + clipboard.dispose(); + clipboard = null; + shell.dispose(); + shell = null; + SwtTestUtil.processEvents(); + + // tearDown doesn't dispose on each test, but the error we are checking + // for happens when Display is disposed (causing ClipboardProxy to be disposed) + display.dispose(); + display = null; + + // start back up + setUp(); + } + + private void checkStderrForGTK3Warning(String stderr) { + if (SwtTestUtil.isGTK3()) { + if ("***WARNING: Attempt to access SWT clipboard after disposing SWT Display.\n".equals(stderr)) { + // test passed (VM didn't SIGSEGV) and we exercise the problematic code causing + // https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + } else if (stderr.isEmpty()) { + assumeTrue(false, """ + test passed (VM didn't SIGSEGV) but we didn't exercise the problematic code causing + https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + """); + } else { + // test passed (VM didn't SIGSEGV) but we don't know why there is other stderr + // output, log it to the test results for further investigation. + assertEquals("", stderr); + } + } else { + assertEquals("", stderr); + } + } + + private Text setContentsOnClipboardAndDisposeDisplay() throws InterruptedException { + openAndFocusShell(false); + + String helloWorld = getUniqueTestString(); + clipboard.setContents(new Object[] { helloWorld }, new Transfer[] { textTransfer }); + assertEquals(helloWorld, clipboard.getContents(textTransfer)); + + tearDownAndStartAgain(); + shell = new Shell(display); + Text text = new Text(shell, SWT.SINGLE); + String textBoxHello = getUniqueTestString(); + text.setText(textBoxHello); + text.setSelection(0, textBoxHello.length()); + + return text; + } + + /** + * Regression test for + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + */ + @Test + public void test_AfterNewDisplay_TextCopy() throws Exception { + Text text = setContentsOnClipboardAndDisposeDisplay(); + SwtTestUtil.openShell(shell); + + /* + * on GTK3 this Text.copy would trigger SIGSEGV on GTK calling disposed callback + * ClipboardProxy.clearFunc + */ + String stderr = SwtTestUtil.runWithCapturedStderr(() -> text.copy()); + checkStderrForGTK3Warning(stderr); + + // After the test, clear out the partially disposed clipboard by taking + // ownership and then clearing it + clipboard.setContents(new Object[] { getUniqueTestString() }, new Transfer[] { textTransfer }); + clipboard.clearContents(); + } + + /** + * Regression test for + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + */ + @Test + public void test_AfterNewDisplay_LocalSetContents() throws Exception { + setContentsOnClipboardAndDisposeDisplay(); + SwtTestUtil.openShell(shell); + + /* + * on GTK3 this setContents would lead to undefined behavior because when the + * ClipboardProxy called bind on the new ClipboardProxy.clearFunc there was a + * pretty good chance it would bind to the same entry in the callback table. But + * depending on exact order of operations that table entry could be empty, + * leading to SIGSEGV. + */ + String stderr = SwtTestUtil.runWithCapturedStderr( + () -> clipboard.setContents(new Object[] { getUniqueTestString() }, new Transfer[] { textTransfer })); + checkStderrForGTK3Warning(stderr); + } + + /** + * Regression test for + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + */ + @Test + public void test_AfterNewDisplay_RemoteGetContents() throws Exception { + setContentsOnClipboardAndDisposeDisplay(); + openAndFocusRemote(); + /* + * on GTK3 this getting from the remote would trigger SIGSEGV on GTK calling + * disposed callback ClipboardProxy.getFunc + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/2675 + * + * Caveat: What is available on the clipboard after Display dispose is very + * platform and configuration dependent and no guarantees in the SWT API exist. + * Therefore the return value of remote.getStringContents() is not checked here. + */ + String stderr = SwtTestUtil + .runWithCapturedStderr(() -> SwtTestUtil.runOperationInThread(() -> remote.getStringContents())); + checkStderrForGTK3Warning(stderr); + + /* + * On GTK3 At this point the old ClipboardProxy has not been disposed because + * this process still owns the clipboard from the original setContents done at + * the beginning of the test. Therefore set some new contents so that the old + * contents can be cleared. + */ + SwtTestUtil.openShell(shell); + stderr = SwtTestUtil.runWithCapturedStderr( + () -> clipboard.setContents(new Object[] { getUniqueTestString() }, new Transfer[] { textTransfer })); + checkStderrForGTK3Warning(stderr); + } }