GNOME Bugzilla – Bug 711779
Fix memory leaks in libgobject tests
Last modified: 2018-05-24 15:55:32 UTC
In order to make libgobject tests be valgrindable, I'll post patches which fix memory leaks. In general it's good practice to free stuff in tests, so that we can run through those code paths, and make sure we received appropriate references/memory.
Created attachment 259434 [details] [review] enums: Fix leaks in tests
Created attachment 259435 [details] [review] threadtests: Fix leaks in tests
Created attachment 259437 [details] [review] param: Avoid g_test_add_data_func_full() which leaks It leaks on tests that have not been run. This is the case when the g_test_subprocess mechanics are in use.
Created attachment 259438 [details] [review] accumulator: Fix leak in test
Created attachment 259439 [details] [review] gvalue-test: Fix leaks in test
Created attachment 259440 [details] [review] override: Fix leaks in test
Created attachment 259441 [details] [review] paramspec-test: Fix leaks in tests
Created attachment 259442 [details] [review] signals: Fix leak in test
Created attachment 259443 [details] [review] timeloop-closure: Fix leaks in test
Created attachment 259444 [details] [review] defaultiface: Fix leak in test
Created attachment 259445 [details] [review] dynamictype: Fix leak in test
Review of attachment 259434 [details] [review]: sure
Review of attachment 259435 [details] [review]: ok
Review of attachment 259437 [details] [review]: ok
Review of attachment 259438 [details] [review]: ok
Review of attachment 259439 [details] [review]: ok
Review of attachment 259440 [details] [review]: ok
Review of attachment 259441 [details] [review]: sure
Review of attachment 259442 [details] [review]: ok
Review of attachment 259437 [details] [review]: This is somewhat unusual - it is the only patch where you had to use cleanup facilities to fix a leak. Are we missing some api here ?
Review of attachment 259437 [details] [review]: .
Review of attachment 259443 [details] [review]: ok
Review of attachment 259444 [details] [review]: yes
Review of attachment 259445 [details] [review]: sure
Thanks. First pass pushing patches that don't need changes. Attachment 259434 [details] pushed as ac6d35b - enums: Fix leaks in tests Attachment 259435 [details] pushed as 5339950 - threadtests: Fix leaks in tests Attachment 259438 [details] pushed as 83301d8 - accumulator: Fix leak in test Attachment 259439 [details] pushed as e6de9c6 - gvalue-test: Fix leaks in test Attachment 259440 [details] pushed as 320f0b3 - override: Fix leaks in test Attachment 259441 [details] pushed as 1b96620 - paramspec-test: Fix leaks in tests Attachment 259442 [details] pushed as d872244 - signals: Fix leak in test Attachment 259443 [details] pushed as bac4179 - timeloop-closure: Fix leaks in test Attachment 259444 [details] pushed as fd7b2fa - defaultiface: Fix leak in test
(In reply to comment #24) > Review of attachment 259445 [details] [review]: > > sure This is actually dependent on a patch in bug #711778. Moved there. (In reply to comment #20) > Review of attachment 259437 [details] [review]: > > This is somewhat unusual - it is the only patch where you had to use cleanup > facilities to fix a leak. > Are we missing some api here ? Indeed. First of all this patch is on the wrong bug. Moved to bug #711778. But yes, g_test_add_data_func_full() uses the teardown functionality to free the fixture data. teardown() isn't run if the test isn't run. Therefore this will always leak. Personally, I don't think using teardown() for freeing fixture data is completely correct. Maybe Dan can shed more light here, was introduced here: commit 0c0cdfd9c4a9d57aae0fb50b5e18cab6ba9e1a76 Author: Dan Winship <danw@gnome.org> Date: Thu Aug 23 12:29:36 2012 -0400 gtestutils: add g_test_add_data_func_full() Note that in the above I refer to Fixture as the static input data to a text. In gtestutils.c and documentation we confuse the modifiable test case structure with the actual const data which is input to the test. This latter is what other unit test suites call a 'fixture'.
Leaving this bug open to receive more test case leak patches. But right now both glib/tests/ and tests/gobject/ run without leaks (given the gcleanup stuff, of course).
Comment on attachment 259444 [details] [review] defaultiface: Fix leak in test This depends on GTypeModule cleanup, so reverted, and moving to that bug.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/783.