From 6379e4648e58db3eca532e133a53942dffbcd035 Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Thu, 18 Sep 2008 11:05:48 +0200 Subject: [PATCH] Added debian/patches/perl_deadlock.dpatch. Upstream patch to fix a possible deadlock in the perl plugin. Closes: #499179 --- debian/changelog | 4 +- debian/patches/00list | 1 + debian/patches/perl_deadlock.dpatch | 78 +++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100755 debian/patches/perl_deadlock.dpatch diff --git a/debian/changelog b/debian/changelog index 97ca2b4..a4aecb2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -4,6 +4,8 @@ collectd (4.4.2-2) unstable; urgency=low in 4.4.1-1 and instead build depend on libopenipmi-dev (>= 2.0.14-1~) which includes fixed .pc files. This fixes an undefined symbol error when loading the ipmi plugin caused by that work around (Closes: #494665). + * Added debian/patches/perl_deadlock.dpatch - upstream patch to fix a + possible deadlock in the perl plugin (Closes: #499179). * Added debian/patches/memory_libstatgrab.dpatch - trivial upstream patch to fix a typo in the libstatgrab code of the memory plugin. * Added debian/patches/collectd_memleak.dpatch - trivial upstream patch to @@ -15,7 +17,7 @@ collectd (4.4.2-2) unstable; urgency=low * Added debian/patches/memcached_timeout.dpatch - trivial upstream patch to fix the timeout passed to poll(2). - -- Sebastian Harl Wed, 17 Sep 2008 20:08:55 +0200 + -- Sebastian Harl Thu, 18 Sep 2008 11:03:50 +0200 collectd (4.4.2-1) unstable; urgency=low diff --git a/debian/patches/00list b/debian/patches/00list index c72403e..066e8e9 100644 --- a/debian/patches/00list +++ b/debian/patches/00list @@ -6,4 +6,5 @@ collectd_memleak.dpatch snmp_memleak.dpatch memcached_fdleak.dpatch memcached_timeout.dpatch +perl_deadlock.dpatch diff --git a/debian/patches/perl_deadlock.dpatch b/debian/patches/perl_deadlock.dpatch new file mode 100755 index 0000000..bc9d3d1 --- /dev/null +++ b/debian/patches/perl_deadlock.dpatch @@ -0,0 +1,78 @@ +#! /bin/sh /usr/share/dpatch/dpatch-run +## perl_deadlock.dpatch by Sebastian Harl +## +## DP: Collectd.pm: Improved the way access to @plugins is synchronized. +## DP: +## DP: So far, a lock has been placed on @plugins, so that no two threads could +## DP: access the list of plugins simultaneously. This could cause problems +## DP: which in certain situations could even lead to deadlocks. E.g. when +## DP: using the perl plugin in combination with the rrdtool plugin with +## DP: debugging enabled one would get hit by the following situation: the perl +## DP: plugin holds the lock on @plugins and then dispatches values to the +## DP: rrdtool plugin from some Perl plugin's read function. The rrdtool plugin +## DP: then tries to acquire its cache lock. At the same time some other plugin +## DP: dispatches values to the rrdtool plugin as well and this thread now +## DP: holds the lock on the rrdtool cache. While holding that lock, the +## DP: rrdtool plugin might dispatch a debug logging message and thus calls the +## DP: perl plugin's log-callback which tries to get the lock on @plugins thus +## DP: causing a deadlock. +## DP: +## DP: This has been resolved by the following two changes: +## DP: +## DP: * Restrict the lock to the list of plugins of one type. This allows to +## DP: access e.g. read and log plugins in parallel. +## DP: +## DP: * Unlock the variable before calling the Perl callback function. This +## DP: further prevents nested locks. +## DP: +## DP: (This is upstream Git commit dffb71cd6668e98f93c12da2ee2bd7a728d7292a) + +@DPATCH@ + +diff a/bindings/perl/Collectd.pm b/bindings/perl/Collectd.pm +--- a/bindings/perl/Collectd.pm ++++ b/bindings/perl/Collectd.pm +@@ -133,6 +133,8 @@ sub DEBUG { _log (scalar caller, LOG_DEBUG, shift); } + sub plugin_call_all { + my $type = shift; + ++ my %plugins; ++ + our $cb_name = undef; + + if (! defined $type) { +@@ -148,9 +150,13 @@ sub plugin_call_all { + return; + } + +- lock @plugins; +- foreach my $plugin (keys %{$plugins[$type]}) { +- my $p = $plugins[$type]->{$plugin}; ++ { ++ lock %{$plugins[$type]}; ++ %plugins = %{$plugins[$type]}; ++ } ++ ++ foreach my $plugin (keys %plugins) { ++ my $p = $plugins{$plugin}; + + my $status = 0; + +@@ -261,7 +267,7 @@ sub plugin_register { + cb_name => $data, + ); + +- lock @plugins; ++ lock %{$plugins[$type]}; + $plugins[$type]->{$name} = \%p; + } + else { +@@ -286,7 +292,7 @@ sub plugin_unregister { + return plugin_unregister_data_set ($name); + } + elsif (defined $plugins[$type]) { +- lock @plugins; ++ lock %{$plugins[$type]}; + delete $plugins[$type]->{$name}; + } + else { -- 2.30.2