File 0005-zypp-fix-removing-packages.patch of Package PackageKit

Overview Repositories Revisions Requests Users Attributes Meta

File 0005-zypp-fix-removing-packages.patch of Package PackageKit (Revision 1a0ccfd6fcd7b81204ca458d9658ef73)

Currently displaying revision 1a0ccfd6fcd7b81204ca458d9658ef73 , Show latest

From 1011d1acf069305b5359b4b7f060aa0781acead8 Mon Sep 17 00:00:00 2001
From: Stephan Kulow <coolo@suse.de>
Date: Sat, 19 Jan 2013 16:17:03 +0100
Subject: [PATCH] zypp: fix removing packages

- added a mutex around zypp trying to avoid crashes that look
  suspiciously like thread issues (e.g.
  https://bugzilla.novell.com/show_bug.cgi?id=745733#c18)
- add some more debug output to track issues others have
- some whitespace cleanup
- fix get_package_by_id to do not match repo alias for installed
  packages
---
 backends/zypp/pk-backend-zypp-private.h |  5 +-
 backends/zypp/pk-backend-zypp.cpp       | 93 +++++++++++++++++++--------------
 backends/zypp/zypp-events.h             |  4 ++
 backends/zypp/zypp-utils.cpp            | 45 +++++++++++-----
 src/pk-backend-job.c                    |  2 +-
 5 files changed, 93 insertions(+), 56 deletions(-)

diff --git a/backends/zypp/pk-backend-zypp-private.h b/backends/zypp/pk-backend-zypp-private.h
index 6d26eb3..5aa0d73 100644
--- a/backends/zypp/pk-backend-zypp-private.h
+++ b/backends/zypp/pk-backend-zypp-private.h
@@ -5,8 +5,7 @@
 #include <map>
 #include <string>
 #include <vector>
-
-// struct PkBackendJob;
+#include <pthread.h>
 
 #include "zypp-events.h"
 
@@ -15,6 +14,8 @@ typedef struct {
   std::vector<std::string> signatures;
   EventDirector eventDirector;
   PkBackendJob *currentJob;
+  
+  pthread_mutex_t zypp_mutex;
 
 } PkBackendZYppPrivate;
 
diff --git a/backends/zypp/pk-backend-zypp.cpp b/backends/zypp/pk-backend-zypp.cpp
index 05a3662..bce60f2 100644
--- a/backends/zypp/pk-backend-zypp.cpp
+++ b/backends/zypp/pk-backend-zypp.cpp
@@ -97,6 +97,16 @@ public:
 };
 
 /**
+ * We do not pretend we're thread safe when all we do is having a huge mutex
+ */
+gboolean
+pk_backend_supports_parallelization (PkBackend *backend)
+{
+        return FALSE;
+}
+
+
+/**
  * pk_backend_get_description:
  */
 const gchar *
@@ -127,6 +137,7 @@ pk_backend_initialize (PkBackend *backend)
 	/* create private area */
 	priv = new PkBackendZYppPrivate;
 	priv->currentJob = 0;
+	priv->zypp_mutex = PTHREAD_MUTEX_INITIALIZER;
 	zypp_logging ();
 
 	g_debug ("zypp_backend_initialize");
@@ -277,12 +288,11 @@ pk_backend_get_groups (PkBackend *backend)
 PkBitfield
 pk_backend_get_filters (PkBackend *backend)
 {
-	return pk_bitfield_from_enums (
-		PK_FILTER_ENUM_INSTALLED,
-		PK_FILTER_ENUM_ARCH,
-		PK_FILTER_ENUM_NEWEST,
-		PK_FILTER_ENUM_SOURCE,
-		-1);
+	return pk_bitfield_from_enums (PK_FILTER_ENUM_INSTALLED,
+				       PK_FILTER_ENUM_ARCH,
+				       PK_FILTER_ENUM_NEWEST,
+				       PK_FILTER_ENUM_SOURCE,
+				       -1);
 }
 
 /*
@@ -298,9 +308,9 @@ backend_get_depends_thread (PkBackendJob *job, GVariant *params, gpointer user_d
 	gchar **package_ids;
 	gboolean recursive;
 	g_variant_get (params, "(t^a&sb)",
-		&_filters,
-		&package_ids,
-		&recursive);
+		       &_filters,
+		       &package_ids,
+		       &recursive);
 
 	pk_backend_job_set_status (job, PK_STATUS_ENUM_QUERY);
 	pk_backend_job_set_percentage (job, 0);
@@ -313,7 +323,7 @@ backend_get_depends_thread (PkBackendJob *job, GVariant *params, gpointer user_d
 		return;
 	}
 	
-	MIL << pk_filter_bitfield_to_string (_filters) << endl;
+	MIL << package_ids[0] << " " << pk_filter_bitfield_to_string (_filters) << endl;
 
 	try
 	{
@@ -393,21 +403,21 @@ backend_get_depends_thread (PkBackendJob *job, GVariant *params, gpointer user_d
 
 		// print dependencies
 		for (map<string, sat::Solvable>::iterator it = caps.begin ();
-				it != caps.end();
-				++it) {
-
+		     it != caps.end();
+		     ++it) {
+			
 			// backup sanity check for no-solvables
 			if (! it->second.name ().c_str() ||
 			    it->second.name ().c_str()[0] == '\0')
 				continue;
-
+			
 			PoolItem item(it->second);
 			PkInfoEnum info = it->second.isSystem () ? PK_INFO_ENUM_INSTALLED : PK_INFO_ENUM_AVAILABLE;
 
 			g_debug ("add dep - '%s' '%s' %d [%s]", it->second.name().c_str(),
-				   info == PK_INFO_ENUM_INSTALLED ? "installed" : "available",
-				   it->second.isSystem(),
-				   zypp_filter_solvable (_filters, it->second) ? "don't add" : "add" );
+				 info == PK_INFO_ENUM_INSTALLED ? "installed" : "available",
+				 it->second.isSystem(),
+				 zypp_filter_solvable (_filters, it->second) ? "don't add" : "add" );
 
 			if (!zypp_filter_solvable (_filters, it->second)) {
 				zypp_backend_package (job, info, it->second,
@@ -445,7 +455,7 @@ backend_get_details_thread (PkBackendJob *job, GVariant *params, gpointer user_d
 
 	gchar **package_ids;
 	g_variant_get (params, "(^a&s)",
-		&package_ids);
+		       &package_ids);
 
 	ZyppJob zjob(job);
 	ZYpp::Ptr zypp = zjob.get_zypp();
@@ -458,13 +468,14 @@ backend_get_details_thread (PkBackendJob *job, GVariant *params, gpointer user_d
 	pk_backend_job_set_status (job, PK_STATUS_ENUM_QUERY);
 
 	for (uint i = 0; package_ids[i]; i++) {
+		MIL << package_ids[i] << endl;
 
 		sat::Solvable solv = zypp_get_package_by_id( package_ids[i] );
 
 		ResObject::constPtr obj = make<ResObject>( solv );
 		if (obj == NULL) {
-			zypp_backend_finished_error (
-				job, PK_ERROR_ENUM_PACKAGE_NOT_FOUND, "couldn't find package");
+			zypp_backend_finished_error (job, PK_ERROR_ENUM_PACKAGE_NOT_FOUND, 
+						     "couldn't find package");
 			return;
 		}
 
@@ -531,8 +542,8 @@ backend_get_distro_upgrades_thread(PkBackendJob *job, GVariant *params, gpointer
 
 	vector<parser::ProductFileData> result;
 	if (!parser::ProductFileReader::scanDir (functor::getAll (back_inserter (result)), "/etc/products.d")) {
-		zypp_backend_finished_error (
-			job, PK_ERROR_ENUM_INTERNAL_ERROR, "Could not parse /etc/products.d");
+		zypp_backend_finished_error (job, PK_ERROR_ENUM_INTERNAL_ERROR, 
+					     "Could not parse /etc/products.d");
 		return;
 	}
 
@@ -569,12 +580,11 @@ pk_backend_get_distro_upgrades (PkBackend *backend, PkBackendJob *job)
 static void
 backend_refresh_cache_thread (PkBackendJob *job, GVariant *params, gpointer user_data)
 {
-	MIL << endl;
-
 	gboolean force;
 	g_variant_get (params, "(b)",
-		&force);
+		       &force);
 
+	MIL << force << endl;
 	ZyppJob zjob(job);
 	ZYpp::Ptr zypp = zjob.get_zypp();
 
@@ -593,7 +603,6 @@ backend_refresh_cache_thread (PkBackendJob *job, GVariant *params, gpointer user
 void
 pk_backend_refresh_cache (PkBackend *backend, PkBackendJob *job, gboolean force)
 {
-	MIL << endl;
 	pk_backend_job_thread_create (job, backend_refresh_cache_thread, NULL, NULL);
 }
 
@@ -626,12 +635,11 @@ check_for_self_update (PkBackend *backend, set<PoolItem> *candidates)
 static void
 backend_get_updates_thread (PkBackendJob *job, GVariant *params, gpointer user_data)
 {
-	MIL << endl;
-
 	PkBitfield _filters;
 	g_variant_get (params, "(t)",
-		&_filters);
+		       &_filters);
 
+	MIL << pk_filter_bitfield_to_string(_filters) << endl;
 	ZyppJob zjob(job);
 	ZYpp::Ptr zypp = zjob.get_zypp();
 
@@ -717,8 +725,8 @@ backend_install_files_thread (PkBackendJob *job, GVariant *params, gpointer user
 	PkBitfield transaction_flags;
 	gchar **full_paths;
 	g_variant_get (params, "(t^a&s)",
-		&transaction_flags,
-		&full_paths);
+		       &transaction_flags,
+		       &full_paths);
 	
 	if (zypp == NULL){
 		pk_backend_job_finished (job);
@@ -1148,6 +1156,7 @@ backend_resolve_thread (PkBackendJob *job, GVariant *params, gpointer user_data)
 	zypp_build_pool (zypp, TRUE);
 
 	for (uint i = 0; search[i]; i++) {
+		MIL << search[i] << " " << pk_filter_bitfield_to_string(_filters) << endl;
 		vector<sat::Solvable> v;
 		
 		/* build a list of packages with this name */
@@ -1175,6 +1184,8 @@ backend_resolve_thread (PkBackendJob *job, GVariant *params, gpointer user_data)
 		/* Filter the list of packages with this name to 'pkgs' */
 		for (vector<sat::Solvable>::iterator it = v.begin (); it != v.end (); ++it) {
 
+			MIL << "found " << *it << endl;
+
 			if (zypp_filter_solvable (_filters, *it) ||
 			    zypp_is_no_solvable(*it))
 				continue;
@@ -1184,6 +1195,7 @@ backend_resolve_thread (PkBackendJob *job, GVariant *params, gpointer user_data)
 			} else if (it->edition() > newest.edition() || Arch::compare(it->arch(), newest.arch()) > 0) {
 				newest = *it;
 			}
+			MIL << "emit " << *it << endl;
 			pkgs.push_back (*it);
 		}
 		
@@ -1192,12 +1204,13 @@ backend_resolve_thread (PkBackendJob *job, GVariant *params, gpointer user_data)
 			/* 'newest' filter support */
 			if (pk_bitfield_contain (_filters, PK_FILTER_ENUM_NEWEST)) {
 				pkgs.clear();
+				MIL << "emit just newest " << newest << endl;
 				pkgs.push_back (newest);
 			} else if (pk_bitfield_contain (_filters, PK_FILTER_ENUM_NOT_NEWEST)) {
 				pkgs.erase (find (pkgs.begin (), pkgs.end(), newest));
 			}
 		}
-
+		
 		zypp_emit_filtered_packages_in_list (job, _filters, pkgs);
 	}
 
@@ -1637,9 +1650,9 @@ backend_repo_set_data_thread (PkBackendJob *job, GVariant *params, gpointer user
 	const gchar *value;
 
 	g_variant_get(params, "(&s&s&s)",
-		&repo_id,
-		&parameter,
-		&value);
+		      &repo_id,
+		      &parameter,
+		      &value);
 
 	ZyppJob zjob(job);
 	ZYpp::Ptr zypp = zjob.get_zypp();
@@ -1770,10 +1783,10 @@ backend_what_provides_thread (PkBackendJob *job, GVariant *params, gpointer user
 	PkBitfield _filters;
 	PkProvidesEnum provides;
 	g_variant_get(params, "(tu^a&s)",
-		&_filters,
-		&provides,
-		&values);
-
+		      &_filters,
+		      &provides,
+		      &values);
+	
 	ZyppJob zjob(job);
 	ZYpp::Ptr zypp = zjob.get_zypp();
 
@@ -1783,7 +1796,7 @@ backend_what_provides_thread (PkBackendJob *job, GVariant *params, gpointer user
 	}
 	pk_backend_job_set_status (job, PK_STATUS_ENUM_QUERY);
 
-	const gchar *search = values[0]; //Fixme - support possible multiple search values (logical OR)
+	const gchar *search = values[0]; //Fixme - support possible multi1ple search values (logical OR)
 	ResPool pool = zypp_build_pool (zypp, true);
 
 	if((provides == PK_PROVIDES_ENUM_HARDWARE_DRIVER) || g_ascii_strcasecmp("drivers_for_attached_hardware", search) == 0) {
diff --git a/backends/zypp/zypp-events.h b/backends/zypp/zypp-events.h
index b18bf94..69777ec 100644
--- a/backends/zypp/zypp-events.h
+++ b/backends/zypp/zypp-events.h
@@ -93,6 +93,10 @@ public:
 			return;
 		}
 		
+		if (percentage > 100) {
+			MIL << "libzypp is silly" << std::endl;
+		}
+
 		_sub_percentage = percentage;
 		pk_backend_job_set_item_progress(_job, _package_id, PK_STATUS_ENUM_UNKNOWN, _sub_percentage);
 	}
diff --git a/backends/zypp/zypp-utils.cpp b/backends/zypp/zypp-utils.cpp
index d898f8c..002c99b 100644
--- a/backends/zypp/zypp-utils.cpp
+++ b/backends/zypp/zypp-utils.cpp
@@ -71,20 +71,28 @@ using zypp::filesystem::PathInfo;
 
 extern PkBackendZYppPrivate *priv;
 
-ZyppJob::ZyppJob(PkBackendJob *job) 
+ZyppJob::ZyppJob(PkBackendJob *job)
 {
+	MIL << "locking zypp" << std::endl;
+	pthread_mutex_lock(&priv->zypp_mutex);
+
 	if (priv->currentJob) {
-		g_error("currentjob is already defined");
+		MIL << "currentjob is already defined - highly impossible" << endl;
 	}
-
+	
+	pk_backend_job_set_locked(job, true);
 	priv->currentJob = job;
 	priv->eventDirector.setJob(job);
 }
 
 ZyppJob::~ZyppJob()
 {
+	if (priv->currentJob)
+		pk_backend_job_set_locked(priv->currentJob, false);
 	priv->currentJob = 0;
 	priv->eventDirector.setJob(0);
+	MIL << "unlocking zypp" << std::endl;
+	pthread_mutex_unlock(&priv->zypp_mutex);
 }
 
 /**
@@ -428,24 +436,35 @@ zypp_get_package_by_id (const gchar *package_id)
 		sat::Solvable pkg = it->satSolvable();
 		MIL << "match " << package_id << " " << pkg << endl;
 
-		if (want_source && !isKind<SrcPackage>(pkg))
+		if (want_source && !isKind<SrcPackage>(pkg)) {
+			MIL << "not a src package\n";
 			continue;
+		}
 
-		if (!want_source && (isKind<SrcPackage>(pkg) || g_strcmp0 (pkg.arch().c_str(), arch)))
+		if (!want_source && (isKind<SrcPackage>(pkg) || g_strcmp0 (pkg.arch().c_str(), arch))) {
+			MIL << "not a matching arch\n";
 			continue;
+		}
 
 		const string &ver = pkg.edition ().asString();
-		if (g_strcmp0 (ver.c_str (), id_parts[PK_PACKAGE_ID_VERSION]))
-			continue;
-		
-		if (!strncmp(id_parts[PK_PACKAGE_ID_DATA], "installed", 9) && !pkg.isSystem())
-			continue;
-
-		if (pkg.isSystem() && strncmp(id_parts[PK_PACKAGE_ID_DATA], "installed", 9))
+		if (g_strcmp0 (ver.c_str (), id_parts[PK_PACKAGE_ID_VERSION])) {
+			MIL << "not a matching version\n";
 			continue;
+		}
 
-		if (g_strcmp0(pkg.repository().alias().c_str(), id_parts[PK_PACKAGE_ID_DATA]))
+		if (!pkg.isSystem()) {
+			if (!strncmp(id_parts[PK_PACKAGE_ID_DATA], "installed", 9)) {
+				MIL << "pkg is not installed\n";
+				continue;
+			}
+			if (g_strcmp0(pkg.repository().alias().c_str(), id_parts[PK_PACKAGE_ID_DATA])) {
+				MIL << "repo does not match\n";
+				continue;
+			}
+		} else if (strncmp(id_parts[PK_PACKAGE_ID_DATA], "installed", 9)) {
+			MIL << "pkg installed\n";
 			continue;
+		}
 
 		MIL << "found " << pkg << endl;
 		package = pkg;
diff --git a/src/pk-backend-job.c b/src/pk-backend-job.c
index 45e992a..d1d7035 100644
--- a/src/pk-backend-job.c
+++ b/src/pk-backend-job.c
@@ -987,7 +987,7 @@ pk_backend_job_set_item_progress (PkBackendJob *job,
 
 	/* invalid number? */
 	if (percentage > 100 && percentage != PK_BACKEND_PERCENTAGE_INVALID) {
-		g_debug ("invalid number %i", percentage);
+		g_error ("invalid number %i", percentage);
 		return;
 	}
 
-- 
1.8.1

Places

File 0005-zypp-fix-removing-packages.patch of Package PackageKit (Revision 1a0ccfd6fcd7b81204ca458d9658ef73)

Places