Bug 30096

Summary: unbalanced ref count for McdRequest is masked by ref leak if there are suitable plugins
Product: Telepathy Reporter: Vivek Dasmohapatra <vivek>
Component: mission-controlAssignee: Vivek Dasmohapatra <vivek>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium Keywords: patch
Version: git master   
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/vivek/telepathy-mission-control;a=shortlog;h=refs/heads/mcd-request-ref-fixes
Whiteboard: review+
i915 platform: i915 features:

Description Vivek Dasmohapatra 2010-09-08 18:39:29 UTC 
The ref count of an McdRequest is decremented along with self->delay 
every time _mcd_request_end_delay is called, and self->delay is initialised 
to 1 in the _init function for McdRequest. 

This means that McdRequest should be initialised with a ref count of 
1 + self->delay by the constructor-helper, but it currently is not
(the _constructed method should probably ref the object before returning 
it to do this).

However

_mcd_account_proceed_with_request() will call:

  _mcd_plugin_request_new (account, _mcd_channel_get_request (channel))

_if_ there are > 0 plugins which satisfy MCP_IS_REQUEST_POLICY (which is 
the case in the test suite): The object created by this holds a ref to 
the McdRequest.

_mcd_account_proceed_with_request() does not unref this object (stored 
in the plugin_api variable).

End result: if there are CP_IS_REQUEST_POLICY plugins, the ref count for
McdRequest objects will be right, for the wrong reasons, and we'll leak 
an McdPluginRequest.

If there are no such plugins (we don't ship any by default, I think)
then McdPluginRequest won't leak, but McdRequest objects will end up
getting freed too early.

Not sure how/if we could construct a test case for this. I guess 
running a test that involved a request in a copy of MC _without_ 
any request policy plugins would demonstrate that things were set
up right.
Comment 1 Simon McVittie 2010-09-09 02:41:34 UTC 
r+.

I have an equivalent branch with more verbose commit messages, which you might like to consider instead, though - in general, essays written in bugs will never be read by a future maintainer, whereas commit messages and code comments continue to be useful.

http://git.collabora.co.uk/?p=user/smcv/telepathy-mission-control-smcv.git;a=shortlog;h=refs/heads/just-what-are-refs

(Also, tp_clear_object() is a good way to make "if (o != NULL) g_object_unref (o)" less verbose.)
Comment 2 Vivek Dasmohapatra 2010-09-09 03:32:25 UTC 
Ok, lets go with your branch & comments.
Comment 3 Simon McVittie 2010-09-09 03:49:29 UTC 
Fixed in git for 5.5.4; bug not present in 5.5.3.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.