Account for yet another X11 race condition in unit tests.

jtanx · Aug 10, 2016 · 8086473 · 8086473
1 parent 83f2da9
commit 8086473
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 6 deletions.
diff --git a/src/clipboard_x11.c b/src/clipboard_x11.c
@@ -185,11 +185,13 @@ static void x11_clear_selection(clipboard_c *cb, xcb_selection_clear_event_t *e)
     }
 
     for (int i = 0; i < LCB_MODE_END; i++) {
-        if (cb->selections[i].xmode == e->selection && (pthread_mutex_lock(&cb->mu) == 0)) {
-            xcb_atom_t xmode = cb->selections[i].xmode;
-            cb->free(cb->selections[i].data);
-            memset(&cb->selections[i], 0, sizeof(selection_c));
-            cb->selections[i].xmode = xmode;
+        selection_c *sel = &cb->selections[i];
+        if (sel->xmode == e->selection && (pthread_mutex_lock(&cb->mu) == 0)) {
+            cb->free(sel->data);
+            sel->data = NULL;
+            sel->length = 0;
+            sel->has_ownership = false;
+            sel->target = XCB_NONE;
             pthread_mutex_unlock(&cb->mu);
             break;
         }

diff --git a/test/libclipboard-test-private.h b/test/libclipboard-test-private.h
@@ -26,7 +26,7 @@
 
 #  define TRY_RUN(fn, ev, ret, oper) do { \
     ret = fn; \
-    for (int i = 0; i < 10 && oper(ret, ev); i++) { \
+    for (int i = 0; i < 5 && oper(ret, ev); i++) { \
         std::cout << "Warning(at line:" TO_STRING(__LINE__) "): " TO_STRING(fn) " returned '" << \
             ret << "' trying again!" << std::endl; \
         std::this_thread::sleep_for(std::chrono::milliseconds(50)); \

diff --git a/test/test_basics.cpp b/test/test_basics.cpp
@@ -77,7 +77,16 @@ TEST_F(BasicsTest, TestOwnership) {
     ASSERT_TRUE(clipboard_set_text_ex(cb1, "test", -1, LCB_CLIPBOARD));
     ASSERT_TRUE(clipboard_has_ownership(cb1, LCB_CLIPBOARD));
 
+    char *ret;
     ASSERT_FALSE(clipboard_has_ownership(cb2, LCB_CLIPBOARD));
+    /*
+       The line below is present only for synchronisation purposes.
+       On X11, it may happen that cb2's set text call happens *before*
+       cb1's, meaning that the ownership would still belong to cb1.
+     */
+    TRY_RUN_EQ(clipboard_text_ex(cb2, NULL, LCB_CLIPBOARD), NULL, ret);
+    ASSERT_TRUE(ret != NULL);
+    free(ret);
     ASSERT_TRUE(clipboard_set_text_ex(cb2, "test2", -1, LCB_CLIPBOARD));
 
     bool has_ownership;