[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#924242: marked as done (unblock: gnome-keyring/3.28.2-5)



Your message dated Sun, 10 Mar 2019 14:53:31 +0000
with message-id <20190310145331.GA24332@powdarrmonkey.net>
and subject line Re: Bug#924242: unblock: gnome-keyring/3.28.2-5
has caused the Debian Bug report #924242,
regarding unblock: gnome-keyring/3.28.2-5
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
924242: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=924242
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package gnome-keyring (again) to fix a race condition in
one of the build-time tests (#909416), which led to intermittent FTBFS
on the buildds and consistent FTBFS on single-core machines. There are
no changes to non-test code.

The attached debdiff is relative to the version that was already unblocked
in #924085, which failed to build on mipsel (probably caused by #909416).

unblock gnome-keyring/3.28.2-5

Thanks,
    smcv
diffstat for gnome-keyring-3.28.2 gnome-keyring-3.28.2

 changelog                                                               |   10 
 patches/series                                                          |    1 
 patches/test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch |  111 ++++++++++
 3 files changed, 122 insertions(+)

diff -Nru gnome-keyring-3.28.2/debian/changelog gnome-keyring-3.28.2/debian/changelog
--- gnome-keyring-3.28.2/debian/changelog	2019-03-09 12:00:46.000000000 +0000
+++ gnome-keyring-3.28.2/debian/changelog	2019-03-10 10:51:58.000000000 +0000
@@ -1,3 +1,13 @@
+gnome-keyring (3.28.2-5) unstable; urgency=medium
+
+  * Team upload
+  * d/p/test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch:
+    Add proposed patch to avoid a race condition that can result in the
+    tests hanging, which seems to be particularly likely on a
+    single-CPU machine (Closes: #909416)
+
+ -- Simon McVittie <smcv@debian.org>  Sun, 10 Mar 2019 10:51:58 +0000
+
 gnome-keyring (3.28.2-4) unstable; urgency=medium
 
   * Team upload
diff -Nru gnome-keyring-3.28.2/debian/patches/series gnome-keyring-3.28.2/debian/patches/series
--- gnome-keyring-3.28.2/debian/patches/series	2019-03-09 12:00:46.000000000 +0000
+++ gnome-keyring-3.28.2/debian/patches/series	2019-03-10 10:51:58.000000000 +0000
@@ -3,3 +3,4 @@
 egg-Write-Proc-Type-header-before-DEK-Info.patch
 secret-store-Sort-fields-alphabetically-before-outputting.patch
 gkm-mock-Also-store-objects-in-the-order-they-are-taken.patch
+test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch
diff -Nru gnome-keyring-3.28.2/debian/patches/test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch gnome-keyring-3.28.2/debian/patches/test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch
--- gnome-keyring-3.28.2/debian/patches/test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch	1970-01-01 01:00:00.000000000 +0100
+++ gnome-keyring-3.28.2/debian/patches/test-gkd-ssh-agent-service-Avoid-race-condition-with-serv.patch	2019-03-10 10:51:58.000000000 +0000
@@ -0,0 +1,111 @@
+From: Simon McVittie <smcv@debian.org>
+Date: Sat, 9 Mar 2019 17:56:55 +0000
+Subject: test-gkd-ssh-agent-service: Avoid race condition with server thread
+
+These tests create a server thread in setup() and join it in teardown(),
+but there are various race conditions between them that can cause the
+test to hang. These are particularly reproducible when building on a
+single-CPU machine or VM, and particularly in the startup_shutdown
+test (which doesn't do anything, so it runs teardown() immediately
+after setup()).
+
+It's possible to get this preemption pattern:
+
+     ___ Main thread ___                ___ Server thread ___
+     g_thread_new()                     (starts)
+     g_cond_wait() (blocks)
+                                        ...
+                                        g_cond_signal()
+                                        (gets preempted here)
+     exit setup()
+     enter teardown()
+     g_main_loop_quit()
+                                        g_main_loop_run()
+
+which means g_main_loop_run() will never terminate, because it wasn't
+running yet when the main thread told the GMainLoop to quit, and the
+main thread won't tell it to quit again.
+
+(I couldn't reproduce this under gdb, because it's a race condition and
+is timing-dependent.)
+
+One way to solve this would be for the server thread to signal
+test->cond from an idle callback instead of directly from
+server_thread(), to guarantee that the GMainLoop is already running.
+However, it seems easier to reason about if we avoid GMainLoop and
+iterate the main context directly.
+
+Signed-off-by: Simon McVittie <smcv@debian.org>
+Bug-Debian: https://bugs.debian.org/909416
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-keyring/merge_requests/12
+---
+ daemon/ssh-agent/test-gkd-ssh-agent-service.c | 23 +++++++++++------------
+ 1 file changed, 11 insertions(+), 12 deletions(-)
+
+diff --git a/daemon/ssh-agent/test-gkd-ssh-agent-service.c b/daemon/ssh-agent/test-gkd-ssh-agent-service.c
+index 9a9ead9..5c7a617 100644
+--- a/daemon/ssh-agent/test-gkd-ssh-agent-service.c
++++ b/daemon/ssh-agent/test-gkd-ssh-agent-service.c
+@@ -38,7 +38,8 @@ typedef struct {
+ 	EggBuffer req;
+ 	EggBuffer resp;
+ 	GkdSshAgentService *service;
+-	GMainLoop *loop;
++	GMainContext *server_thread_context;
++	volatile gint server_thread_stop;
+ 	GSocketConnection *connection;
+ 	GThread *thread;
+ 	GMutex lock;
+@@ -49,13 +50,9 @@ static gpointer
+ server_thread (gpointer data)
+ {
+ 	Test *test = data;
+-	GMainContext *context;
+ 	gboolean ret;
+ 
+-	context = g_main_context_new ();
+-	test->loop = g_main_loop_new (context, FALSE);
+-
+-	g_main_context_push_thread_default (context);
++	g_main_context_push_thread_default (test->server_thread_context);
+ 
+ 	ret = gkd_ssh_agent_service_start (test->service);
+ 	g_assert_true (ret);
+@@ -64,12 +61,10 @@ server_thread (gpointer data)
+ 	g_cond_signal (&test->cond);
+ 	g_mutex_unlock (&test->lock);
+ 
+-	g_main_loop_run (test->loop);
++	while (g_atomic_int_get (&test->server_thread_stop) == 0)
++		g_main_context_iteration (test->server_thread_context, TRUE);
+ 
+-	g_main_context_pop_thread_default (context);
+-
+-	g_main_context_unref (context);
+-	g_main_loop_unref (test->loop);
++	g_main_context_pop_thread_default (test->server_thread_context);
+ 
+ 	return NULL;
+ }
+@@ -139,6 +134,7 @@ setup (Test *test, gconstpointer unused)
+ 
+ 	g_mutex_init (&test->lock);
+ 	g_cond_init (&test->cond);
++	test->server_thread_context = g_main_context_new ();
+ 
+ 	test->thread = g_thread_new ("ssh-agent", server_thread, test);
+ 
+@@ -151,9 +147,12 @@ setup (Test *test, gconstpointer unused)
+ static void
+ teardown (Test *test, gconstpointer unused)
+ {
+-	g_main_loop_quit (test->loop);
++	g_atomic_int_set (&test->server_thread_stop, 1);
++	g_main_context_wakeup (test->server_thread_context);
+ 	g_thread_join (test->thread);
+ 
++	g_main_context_unref (test->server_thread_context);
++
+ 	g_clear_object (&test->connection);
+ 
+ 	gkd_ssh_agent_service_stop (test->service);

--- End Message ---
--- Begin Message ---
On Sun, Mar 10, 2019 at 02:48:10PM +0000, Simon McVittie wrote:
> Please unblock package gnome-keyring (again) to fix a race condition in
> one of the build-time tests (#909416), which led to intermittent FTBFS
> on the buildds and consistent FTBFS on single-core machines. There are
> no changes to non-test code.

Updated existing hint; thanks.


-- 
Jonathan Wiltshire                                      jmw@debian.org
Debian Developer                         http://people.debian.org/~jmw

4096R: 0xD3524C51 / 0A55 B7C5 1223 3942 86EC  74C3 5394 479D D352 4C51

--- End Message ---

Reply to: