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 666115 - various tests leak memory, obscuring real leaks in the library
various tests leak memory, obscuring real leaks in the library
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: general
2.31.x
Other Linux
: Normal minor
: ---
Assigned To: Simon McVittie
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-12-13 19:28 UTC by Simon McVittie
Modified: 2018-05-24 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tls-interaction test: use a weak pointer instead of a deliberate use-after-free (1.05 KB, patch)
2011-12-13 19:30 UTC, Simon McVittie
accepted-commit_now Details | Review
testglib: test_file_functions: don't close fd if it's -1 (829 bytes, patch)
2011-12-13 19:30 UTC, Simon McVittie
accepted-commit_now Details | Review
various GLib tests: plug memory leaks (19.85 KB, patch)
2011-12-13 19:32 UTC, Simon McVittie
accepted-commit_now Details | Review
GOptionContext test: free all arguments, not just the remaining ones (33.26 KB, patch)
2011-12-13 19:32 UTC, Simon McVittie
accepted-commit_now Details | Review
hash test: avoid leaking various keys and values (3.23 KB, patch)
2011-12-13 19:32 UTC, Simon McVittie
accepted-commit_now Details | Review
Plug some leaks in the GIO tests (3.36 KB, patch)
2011-12-13 19:32 UTC, Simon McVittie
accepted-commit_now Details | Review
Add a g_object_assert_last_unref macro (2.75 KB, patch)
2011-12-14 16:43 UTC, Simon McVittie
reviewed Details | Review

Description Simon McVittie 2011-12-13 19:28:53 UTC
If Bug #666114 is fixed, real leaks like those in Bug #666113 are made harder to see among harmless leaks and other misbehaviour from the tests themselves. Patches to follow.
Comment 1 Simon McVittie 2011-12-13 19:30:27 UTC
Created attachment 203378 [details] [review]
tls-interaction test: use a weak pointer instead of a  deliberate use-after-free

---

The intention seems to be to check that the object is actually finalized; weak pointers seem a better way to do that.
Comment 2 Simon McVittie 2011-12-13 19:30:56 UTC
Created attachment 203379 [details] [review]
testglib: test_file_functions: don't close fd if it's -1

---

Causes harmless noise from valgrind.
Comment 3 Simon McVittie 2011-12-13 19:32:01 UTC
Created attachment 203380 [details] [review]
various GLib tests: plug memory leaks

---

These are the relatively easy/obvious ones; if any of them are controversial, I'll split them into a separate commit.
Comment 4 Simon McVittie 2011-12-13 19:32:18 UTC
Created attachment 203381 [details] [review]
GOptionContext test: free all arguments, not just the  remaining ones

On success, g_option_context_parse alters argv by removing options that
it understood, so g_strfreev is insufficient. Instead, take a shallow
copy and free all of the arguments in that, then free the array argv
but not its contents.

Also, improve the checks in error cases, by checking that argv has
not been altered in this way.
Comment 5 Simon McVittie 2011-12-13 19:32:34 UTC
Created attachment 203382 [details] [review]
hash test: avoid leaking various keys and values
Comment 6 Simon McVittie 2011-12-13 19:32:52 UTC
Created attachment 203383 [details] [review]
Plug some leaks in the GIO tests
Comment 7 Matthias Clasen 2011-12-13 22:36:50 UTC
Review of attachment 203380 [details] [review]:

Wow, quite a lot of work here, thanks. I'll just assume that these are all fine...
Comment 8 Matthias Clasen 2011-12-13 22:37:03 UTC
Review of attachment 203379 [details] [review]:

Sure
Comment 9 Matthias Clasen 2011-12-13 22:44:55 UTC
Review of attachment 203381 [details] [review]:

hmm, ok
Comment 10 Matthias Clasen 2011-12-13 23:06:06 UTC
Review of attachment 203382 [details] [review]:

quite a bit more complicated. but ok, I guess, in the name of leakfreeness
Comment 11 Matthias Clasen 2011-12-13 23:06:52 UTC
Review of attachment 203383 [details] [review]:

ok
Comment 12 Simon McVittie 2011-12-14 12:37:30 UTC
(In reply to comment #1)
> Created an attachment (id=203378)
> tls-interaction test: use a weak pointer instead of a  deliberate
> use-after-free

Thanks for reviewing, I'll get those committed. Any thoughts on this one?
Comment 13 Simon McVittie 2011-12-14 12:54:53 UTC
(In reply to comment #12)
> Thanks for reviewing, I'll get those committed.

All patches except Attachment #203378 [details] pushed to master.
Comment 14 Dan Winship 2011-12-14 14:07:30 UTC
Comment on attachment 203378 [details] [review]
tls-interaction test: use a weak pointer instead of a  deliberate use-after-free

yeah, I fixed a bunch of these the same way in glib-networking too. (Though I was thinking maybe it would be nice to have a g_object_assert_last_unref() macro maybe...)
Comment 15 Simon McVittie 2011-12-14 16:43:17 UTC
Created attachment 203492 [details] [review]
Add a g_object_assert_last_unref macro

---

(In reply to comment #14)
> (From update of attachment 203378 [details] [review])
> yeah, I fixed a bunch of these the same way in glib-networking too.

Thanks, pushed as a1bd6e0717.

> (Though I
> was thinking maybe it would be nice to have a g_object_assert_last_unref()
> macro maybe...)

Here you go!
Comment 16 Dan Winship 2012-08-22 19:43:10 UTC
Comment on attachment 203492 [details] [review]
Add a g_object_assert_last_unref macro

just noticed this lying around... looks good to me
Comment 17 Colin Walters 2012-08-28 01:56:42 UTC
Review of attachment 203492 [details] [review]:

::: gobject/gobject.h
@@ +587,3 @@
+ * this macro just calls g_object_unref() without any further checks.
+ *
+ * This macro should only be used in regression tests.

Since: 2.34

@@ +597,3 @@
+    gpointer _weak_pointer = (object); \
+      \
+    g_assert (G_IS_OBJECT (_weak_pointer)); \

Do we need this assertion? g_object_add_weak_pointer() repeats it.
Comment 18 GNOME Infrastructure Team 2018-05-24 13:37:07 UTC
-- 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/488.