Code

Added debian/patches/perl_deadlock.dpatch.
authorSebastian Harl <sh@tokkee.org>
Thu, 18 Sep 2008 09:05:48 +0000 (11:05 +0200)
committerSebastian Harl <sh@tokkee.org>
Thu, 18 Sep 2008 09:05:48 +0000 (11:05 +0200)
Upstream patch to fix a possible deadlock in the perl plugin.

Closes: #499179
debian/changelog
debian/patches/00list
debian/patches/perl_deadlock.dpatch [new file with mode: 0755]

index 97ca2b47a3111757845d92a6a7164b996f9e4272..a4aecb26a0bcfd16ef10715ecc0d4eb4fb7c8936 100644 (file)
@@ -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 <sh@tokkee.org>  Wed, 17 Sep 2008 20:08:55 +0200
+ -- Sebastian Harl <sh@tokkee.org>  Thu, 18 Sep 2008 11:03:50 +0200
 
 collectd (4.4.2-1) unstable; urgency=low
 
index c72403e1513ba3a31052bfcb3f208725d86da29f..066e8e9e8d2dd3ecaefaf089810d24a1b7e28777 100644 (file)
@@ -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 (executable)
index 0000000..bc9d3d1
--- /dev/null
@@ -0,0 +1,78 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## perl_deadlock.dpatch by Sebastian Harl <sh@tokkee.org>
+##
+## 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 {