After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 556387 - FileEnumerator::next_file reference counting problems
FileEnumerator::next_file reference counting problems
Status: RESOLVED FIXED
Product: glibmm
Classification: Bindings
Component: giomm
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2008-10-15 09:54 UTC by Armin Burgmeier
Modified: 2008-10-15 15:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test demonstrating the problem (502 bytes, text/plain)
2008-10-15 09:57 UTC, Armin Burgmeier
  Details
Proposed patch (2.20 KB, patch)
2008-10-15 10:07 UTC, Armin Burgmeier
committed Details | Review

Description Armin Burgmeier 2008-10-15 09:54:23 UTC
The FileEnumerator::next_file method adds an additional reference to the return value via the refreturn flag to gmmproc, but the C version g_file_enumerator_next_file() returns a new object with reference count 1, so there is no need to add an additional reference. This prevents the file enumerator from being freed correctly when the RefPtr goes out of scope.

Similarly, FileEnumerator::next_files_finish() returns a list with shallow ownership, but the C documentation says

"You must free the list with g_list_free() and unref the infos with g_object_unref when you're done with them."

which means we need to set deep ownership for the GFileInfos to get freed correctly.
Comment 1 Armin Burgmeier 2008-10-15 09:57:57 UTC
Created attachment 120631 [details]
Test demonstrating the problem

This demonstrates the problem: When using the C++ version of next_file, the reference count of the resulting GFileInfo object is 2, but it is only 1 using the C version.
Comment 2 Armin Burgmeier 2008-10-15 10:07:15 UTC
Created attachment 120632 [details] [review]
Proposed patch

This patch fixes the problem by not adding an extra reference in next_file and using deep ownership for next_files_finish.
Comment 3 Jonathon Jongsma 2008-10-15 12:17:14 UTC
Thanks, this looks good.  Feel free to commit.
Comment 4 Armin Burgmeier 2008-10-15 12:26:59 UTC
Committed.

2008-10-15  Armin Burgmeier  <armin@openismus.com>

	* gio/src/fileenumerator.hg:
	* gio/src/fileenumerator.ccg: Made FileEnumerator::next_file not add
	an additional reference to the return value, because the C version
	creates a new object. Also, changed ownership of the list returned by
	FileEnumerator::next_files_finish to be deep instead of shallow. Bug
	#556387.
Comment 5 Murray Cumming 2008-10-15 14:44:36 UTC
I guess this isn't documented properly for the C function:
http://library.gnome.org/devel/gio/unstable/GFileEnumerator.html#g-file-enumerator-next-file
Maybe it should say "you should unreference this with g_object_unref()" or suchlike. Could you file a bug for that, please?
Comment 6 Armin Burgmeier 2008-10-15 15:30:54 UTC
I filed bug #556422.