[packagekit] pk_backend_finished race issue

[Richard Hughes]

On 17/09/2007, Tom Parker <palfrey at tevp.net> wrote:
> Richard Hughes wrote:
> > On 14/09/2007, Tom Parker <palfrey at tevp.net> wrote:
> >> The second, I'm not
> >> sure how to fix, and it happens every time. See test.log for the full
> >> log, but the "GThread-ERROR **: file
> >> /tmp/buildd/glib2.0-2.14.0/gthread/gthread-posix.c: line 385 (): error
> >> 'Resource deadlock avoided' during 'pthread_join (*(pthread_t*)thread,
> >> &ignore)'" at the bottom is not a healthy thing.
> >
> > This sort of thing needs to be added to a test-case and tested in make
> > check as it's very difficult to get right. I'll have some time on
> > Sunday to look at this, but until then I'll hold off committing.
>
> I've attached a modified version of pk-self-test.c that causes the same
> problem, without requiring the apt backend, so everyone else can test
> regardless of what backend they're using (and something that can
> eventually be bludgeoned into being a "make check" test).

Quality, I think I've found the issue. We have a nice race on thread
exit and completion, and so the obvious thing to do is to do the
finished (and then wait for threads) in the main thread and then just
do the data in the second thread, i.e.

/**
 * backend_search_group_thread:
 */
static gboolean
backend_search_group_thread (PkBackend *backend, gpointer data)
{
	pk_backend_package (backend, 1, "glib2;2.14.0;i386;fedora",
			 "The GLib library");
	pk_backend_package (backend, 1, "gtk2;gtk2-2.11.6-6.fc8;i386;fedora",
			 "GTK+ Libraries for GIMP");
	return TRUE;
}

/**
 * backend_search_group:
 */
static void
backend_search_group (PkBackend *backend, const gchar *filter, const
gchar *search)
{
	g_return_if_fail (backend != NULL);
	if (pk_backend_thread_create (backend, backend_search_group_thread,
NULL) == FALSE) {
		pk_backend_error_code (backend, PK_ERROR_ENUM_CREATE_THREAD_FAILED,
"Failed to create search-group thread");
		pk_backend_finished (backend, PK_EXIT_ENUM_FAILED);
	}
	pk_backend_finished (backend, PK_EXIT_ENUM_SUCCESS);
}

I've fixed this in my local git tree but i can't push as the whole
colo is down. Ken is sorting it out, and it should be up soon. When it
does come up I've got a huge number of patches to push, along with
some nice new functionality, so please hold off committing to src/
until I get my mega-patch pushed. :-)

 b/backends/box/helpers/Makefile.am        |    2
 b/backends/conary/helpers/Makefile.am     |    2
 b/backends/test/Makefile.am               |   37 +++
 b/backends/test/helpers/Makefile.am       |   15 +
 b/backends/test/helpers/search-name.sh    |   19 +
 b/backends/test/pk-backend-test-fail.c    |  229 ++++++++++++++++++++++
 b/backends/test/pk-backend-test-nop.c     |   50 ++++
 b/backends/test/pk-backend-test-spawn.c   |   63 ++++++
 b/backends/test/pk-backend-test-succeed.c |  247 ++++++++++++++++++++++++
 b/backends/test/pk-backend-test-thread.c  |  118 +++++++++++
 b/backends/yum/helpers/Makefile.am        |    2
 b/client/pk-monitor.c                     |    1
 b/configure.ac                            |    1
 b/libpackagekit/pk-enum-list.c            |   29 ++
 b/libpackagekit/pk-job-list.c             |    4
 b/libpackagekit/pk-task-list.c            |    6
 b/src/Makefile.am                         |    4
 b/src/pk-backend.c                        |  170 +++++++++++-----
 b/src/pk-backend.h                        |    7
 b/src/pk-engine.c                         |   61 ++++--
 b/src/pk-marshal.list                     |    1
 b/src/pk-self-test.c                      |    7
 b/src/pk-thread-list.c                    |  302 ++++++++++++++++++++++++++++++
 b/src/pk-thread-list.h                    |   63 ++++++
 b/src/pk-transaction-db.c                 |  220 +++++++++++++++++++--
 b/src/pk-transaction-db.h                 |   11 +
 backends/test/pk-backend-test.c           |  247 ------------------------
 27 files changed, 1571 insertions(+), 347 deletions(-)

I'll change the apt and box backends to the new scheme also, and then
things should "just work" :-)

Thanks for the help, and hopefully we've fixed this for good.

Richard.