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 749036 - FileSystem objects are memory leaked in init_filesystems()
FileSystem objects are memory leaked in init_filesystems()
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
GIT HEAD
Other Linux
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2015-05-06 19:31 UTC by Mike Fleetwood
Modified: 2015-08-03 17:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
FileSystem object leak fix (v1) (16.58 KB, patch)
2015-05-06 20:33 UTC, Mike Fleetwood
none Details | Review
get_custom_text tidyup (v1) (8.97 KB, patch)
2015-05-17 11:57 UTC, Mike Fleetwood
none Details | Review

Description Mike Fleetwood 2015-05-06 19:31:57 UTC
While looking into other stuff I realised that the FileSystem derived
objects stored in the FILESYSTEM_MAP are allocated once using new in
init_filesystems() but never deleted.

Fix to follow.

Mike
Comment 1 Mike Fleetwood 2015-05-06 20:33:33 UTC
Created attachment 302996 [details] [review]
FileSystem object leak fix (v1)

Hi Curtis,

Here's patchset v1 for this.

Looking back at the code before this commit [1] the same leak existed.
That is on exit the FileSystem objects are never deleted.  It is with
the older code that on every refresh the current objects were deleted
and new ones created in file_supported_filesystems() with the final set
never being deleted.

[1] Initialise file system objects only once
https://git.gnome.org/browse/gparted/commit/?id=5f6656f267151b452cc95e036f8ad138a153d397

Also includes 3 tidy-up patches.

Thanks,
Mike
Comment 2 Curtis Gedak 2015-05-07 14:57:02 UTC
Thanks Mike for catching this memory leak and cleaning up some of the method names.

Patch set v1 in comment #1 looks good to me.  I performed some random testing with gpt and msdos partition tables on kubuntu 12.04, debian 8, and fedora 21 and all went well.  Hence I have committed patch set v1 to be included in the next release of GParted.

The relevant git commits can be viewed at the following links:

Fix memory leak of FileSystem objects in init_filesystems() (#749036)
https://git.gnome.org/browse/gparted/commit/?id=40820bada7b672377450705891308883b5e0bc6d

Tidy-up GParted_Core::init/fini_filesystems() function declarations
https://git.gnome.org/browse/gparted/commit/?id=df000a94a620ee195abfcd2090718f33de70ec81

Make GParted_Core methods flush_device(), get_device(), etc static
https://git.gnome.org/browse/gparted/commit/?id=42cd956a541edf1fc4fab808ac0dd76bcd4843b2

Rename two GParted_Core methods to detect_filesystem*()
https://git.gnome.org/browse/gparted/commit/?id=d0580d59550a595bd53ed1144a7d7cca3cfb7f84
Comment 3 Mike Fleetwood 2015-05-17 11:57:56 UTC
Created attachment 303477 [details] [review]
get_custom_text tidyup (v1)

Hi Curtis,

I have another small tidy patchset.  Seems like overkill to raise a
separate bug report for trivial stuff like this.

Thanks,
Mike
Comment 4 Curtis Gedak 2015-05-23 15:20:10 UTC
Thank you Mike for these latest tidy-up patches.  The patch set looks good to me, and it passed my random testing on kubuntu 12.04.

The patch set in comment #3 has been committed to the git repository for inclusion in the next release of GParted.

The relevant git commits can be viewed at the following links:

Add const qualifier to get_custom_text() member functions
https://git.gnome.org/browse/gparted/commit/?id=f6e4390aaf37229e3f027b91fcdffd6045cb01e2

Small simplification of Win_GParted code which calls get_custom_text()
https://git.gnome.org/browse/gparted/commit/?id=9ced6b051e132da39b441600fc0ecda5971bf8e4
Comment 5 Curtis Gedak 2015-08-03 17:30:25 UTC
This enhancement was included in the GParted 0.23.0 release on August 3, 2015.