| Subject: | Memory leak with Async messages |
Net::DBus leaks when async calls are used.
Devel::Leak::Object shows leaks of the following objects:
Net::DBus::ASyncReply
Net::DBus::Binding::PendingCall
application internal DeferredReply object
application internal Request object
This appears to be due to two reasons:
1. Net::DBus::ASyncReply holds a reference to an instance of
Net::DBus::Binding::PendingCall. When Net::DBus::ASyncReply::set_notify
is called, it does
$self->{pending_call}->set_notify(sub {
my $pending_call = shift;
&$cb($self);
});
this creates a reference from Net::DBus::Binding::PendingCall back to
the Net::DBus::ASyncReply instance via the anonymous closure.
The get_result and discard_result methods in Net::DBus::ASyncReply are
documented as "final" methods, the POD says "After calling this method,
this object should not be used again" for both. Therefore, we can break
the circular reference by deleting the $self->{pending_call} reference
in these methods.
This fix addresses the Net::DBus::Binding::PendingCall leak, but not the
Net::DBus::ASyncReply leak.
2. The code reference passed to
Net::DBus::Binding::PendingCall::set_notify is passed to
Net::DBus::Binding::C::PendingCall::_set_notify, which adds a reference
to the code object and passes it to the DBus library via
dbus_pending_call_set_notify.
The helper methods _pending_call_callback and
_pending_call_notify_release are registered to process the callback and
to free the user data, respectively. The _pending_call_notify_release
method removes the reference to the code object added by _set_notify,
but was never called (an added debug message in this function did not
appear in the log, even though the
Net::DBus::Binding::C::PendingCall::DESTROY debug message did appear),
implying a reference count issue with the C DBusPendingCall object.
The implementation of _pending_call_callback has a
dbus_pending_call_ref(call) statement that appears to be out of place,
as this is a use of an existing reference, not a new reference. Removing
this statement results in _pending_call_notify_release being called and
addresses the Net::DBus::ASyncReply leak, along with the app-specific
DeferredReply and Request object leaks (which were via the closure
passed to Net::DBus::ASyncReply::set_notify).
Originally observed this in the CentOS 5.4 shipped Net::DBus lib
(0.32?), debugged in 0.33.6. Same code is present in 1.0.0.
Tested on perl v5.8.8.
I have been running with the attached patch for months under heavy
workloads without issue.
| Subject: | perl-Net-DBus-leak.patch |
diff -rup Net-DBus-0.33.6/DBus.xs Net-DBus-0.33.6-leakfix/DBus.xs
--- Net-DBus-0.33.6/DBus.xs 2008-02-20 19:26:44.000000000 -0500
+++ Net-DBus-0.33.6-leakfix/DBus.xs 2010-12-01 13:44:56.000000000 -0500
@@ -343,7 +343,8 @@ _pending_call_callback(DBusPendingCall *
selfref = (SV*)dbus_pending_call_get_data(call, pending_call_data_slot);
self = (HV*)SvRV(selfref);
- dbus_pending_call_ref(call);
+ // Why was this here? It makes the call object leak. - FCS
+ //dbus_pending_call_ref(call);
ENTER;
SAVETMPS;
@@ -365,6 +366,7 @@ _filter_release(void *data) {
void
_pending_call_notify_release(void *data) {
+ DEBUG_MSG("In pending call notify release %p\n", data);
SvREFCNT_dec(data);
}
@@ -1189,7 +1191,7 @@ void
DESTROY (call)
DBusPendingCall *call;
CODE:
- DEBUG_MSG("Unrefing pending call %p", call);
+ DEBUG_MSG("Unrefing pending call %p\n", call);
dbus_pending_call_unref(call);
MODULE = Net::DBus::Binding::C::Watch PACKAGE = Net::DBus::Binding::C::Watch
diff -rup Net-DBus-0.33.6/lib/Net/DBus/ASyncReply.pm Net-DBus-0.33.6-leakfix/lib/Net/DBus/ASyncReply.pm
--- Net-DBus-0.33.6/lib/Net/DBus/ASyncReply.pm 2008-02-20 19:26:44.000000000 -0500
+++ Net-DBus-0.33.6-leakfix/lib/Net/DBus/ASyncReply.pm 2010-12-01 12:39:42.000000000 -0500
@@ -84,8 +84,9 @@ calling this method, this object should
sub discard_result {
my $self = shift;
+ my $pending_call = delete $self->{pending_call};
- $self->{pending_call}->cancel;
+ $pending_call->cancel;
}
@@ -151,8 +152,9 @@ object should no longer be used.
sub get_result {
my $self = shift;
+ my $pending_call = delete $self->{pending_call};
- my $reply = $self->{pending_call}->get_reply;
+ my $reply = $pending_call->get_reply;
if ($reply->isa("Net::DBus::Binding::Message::Error")) {
my $iter = $reply->iterator();