Backport patch to fix leaking log handlers in Sack

This commit is contained in:
Aleš Matěj 2019-10-22 11:54:48 +02:00
parent c1eff5f3df
commit c2670737b9
2 changed files with 98 additions and 2 deletions

View File

@ -0,0 +1,92 @@
From 4503d8c017b1c625d65b22279a7e136ec962984c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= <lhrazky@redhat.com>
Date: Mon, 7 Oct 2019 16:33:48 +0200
Subject: [PATCH] Fix leaking log handlers in Sack (RhBug:1758737)
Stores the log handler ids in the sack and uses g_log_remove_handler()
in the sack destructor to remove the handlers.
The mechanism is a bit complex and is explained in a code comment.
https://bugzilla.redhat.com/show_bug.cgi?id=1758737
---
python/hawkey/sack-py.cpp | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/python/hawkey/sack-py.cpp b/python/hawkey/sack-py.cpp
index 5e39dda69..5a1c10d1a 100644
--- a/python/hawkey/sack-py.cpp
+++ b/python/hawkey/sack-py.cpp
@@ -52,6 +52,22 @@ typedef struct {
PyObject *custom_package_class;
PyObject *custom_package_val;
PyObject * ModulePackageContainerPy;
+
+ // g_log handler IDs
+ // Multiple sacks can be created during a run of an application and each
+ // sack opens a log file and registers two g_log handlers. To avoid dangling
+ // handlers with invalid FILE pointers (we close them when destroying the
+ // sack), we need to keep track of the handlers so that we can also remove
+ // them.
+ //
+ // g_log is clever about adding log handlers. It does store all handlers
+ // registered for a given domain, but only the one that was registered last
+ // is used. If you remove the last registered one, the next in line will be
+ // used. That means stacking sacks is ok, the handler from the last
+ // undeleted sack will be the one that is used.
+ guint default_log_handler_id;
+ guint libdnf_log_handler_id;
+
FILE *log_out;
} _SackObject;
@@ -131,8 +147,13 @@ sack_dealloc(_SackObject *o)
}
g_object_unref(o->sack);
}
- if (o->log_out)
+
+ if (o->log_out) {
+ g_log_remove_handler(nullptr, o->default_log_handler_id);
+ g_log_remove_handler("libdnf", o->libdnf_log_handler_id);
fclose(o->log_out);
+ }
+
Py_TYPE(o)->tp_free(o);
}
@@ -194,11 +215,11 @@ log_handler_noop(const gchar *, GLogLevelFlags, const gchar *, gpointer)
}
static gboolean
-set_logfile(const gchar *path, FILE ** log_out, bool debug)
+sack_set_logfile(_SackObject *self, const gchar *path, bool debug)
{
- *log_out = fopen(path, "a");
+ self->log_out = fopen(path, "a");
- if (!(*log_out))
+ if (!self->log_out)
return FALSE;
// The default log handler prints messages that weren't handled by any
@@ -213,8 +234,8 @@ set_logfile(const gchar *path, FILE ** log_out, bool debug)
G_LOG_LEVEL_ERROR);
// set the handler for the default domain as well as "libdnf"
- g_log_set_handler(nullptr, log_mask, log_handler, *log_out);
- g_log_set_handler("libdnf", log_mask, log_handler, *log_out);
+ self->default_log_handler_id = g_log_set_handler(nullptr, log_mask, log_handler, self->log_out);
+ self->libdnf_log_handler_id = g_log_set_handler("libdnf", log_mask, log_handler, self->log_out);
g_info("=== Started libdnf-%d.%d.%d ===", LIBDNF_MAJOR_VERSION,
LIBDNF_MINOR_VERSION, LIBDNF_MICRO_VERSION);
@@ -273,7 +294,7 @@ sack_init(_SackObject *self, PyObject *args, PyObject *kwds)
PycompString logfile(logfile_py);
if (!logfile.getCString())
return -1;
- if (!set_logfile(logfile.getCString(), &self->log_out, debug)) {
+ if (!sack_set_logfile(self, logfile.getCString(), debug)) {
PyErr_Format(PyExc_IOError, "Failed to open log file: %s", logfile.getCString());
return -1;
}

View File

@ -44,13 +44,14 @@
Name: libdnf Name: libdnf
Version: 0.35.5 Version: 0.35.5
Release: 3%{?dist} Release: 4%{?dist}
Summary: Library providing simplified C and Python API to libsolv Summary: Library providing simplified C and Python API to libsolv
License: LGPLv2+ License: LGPLv2+
URL: https://github.com/rpm-software-management/libdnf URL: https://github.com/rpm-software-management/libdnf
Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz
Patch0001: 0001-Revert-9309e92332241ff1113433057c969cebf127734e.patch Patch0001: 0001-Revert-9309e92332241ff1113433057c969cebf127734e.patch
Patch0002: 0001-Use-POOL_FLAG_WHATPROVIDESWITHDISABLED.patch Patch0002: 0001-Use-POOL_FLAG_WHATPROVIDESWITHDISABLED.patch
Patch0003: 0003-Fix-leaking-log-handlers-in-Sack.patch
BuildRequires: cmake BuildRequires: cmake
BuildRequires: gcc BuildRequires: gcc
@ -284,6 +285,9 @@ popd
%endif %endif
%changelog %changelog
* Tue Oct 22 2019 Ales Matej <amatej@redhat.com> - 0.35.5-4
- Fix leaking log handlers in Sack that can cause a crash (RhBug:1758737)
* Mon Oct 14 2019 Jaroslav Mracek <jmracek@redhat.com> - 0.35.5-3 * Mon Oct 14 2019 Jaroslav Mracek <jmracek@redhat.com> - 0.35.5-3
- Add POOL_FLAG_WHATPROVIDESWITHDISABLED flag into pool. - Add POOL_FLAG_WHATPROVIDESWITHDISABLED flag into pool.
- Resolves: 1737469 - Resolves: 1737469
@ -312,7 +316,7 @@ popd
* Wed Sep 11 2019 Jaroslav Mracek <jmracek@redhat.com> - 0.35.3-4 * Wed Sep 11 2019 Jaroslav Mracek <jmracek@redhat.com> - 0.35.3-4
- Backport patch to fix reinstalling packages with a different buildtime - part II - Backport patch to fix reinstalling packages with a different buildtime - part II
* Thu Sep 10 2019 Jaroslav Mracek <jmracek@redhat.com> - 0.35.3-3 * Tue Sep 10 2019 Jaroslav Mracek <jmracek@redhat.com> - 0.35.3-3
- Backport patch to fix reinstalling packages with a different buildtime - Backport patch to fix reinstalling packages with a different buildtime
* Thu Aug 15 2019 Miro Hrončok <mhroncok@redhat.com> - 0.35.3-2 * Thu Aug 15 2019 Miro Hrončok <mhroncok@redhat.com> - 0.35.3-2