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 746652 - gst.supp improvements
gst.supp improvements
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: common
unspecified
Other Linux
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-23 15:07 UTC by Guillaume Desmottes
Modified: 2015-04-23 09:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gst.supp: add some closure related entries (1.23 KB, patch)
2015-03-23 15:07 UTC, Guillaume Desmottes
committed Details | Review
gst.supp: update gst-libav suppression (1.50 KB, patch)
2015-03-26 10:39 UTC, Guillaume Desmottes
accepted-commit_now Details | Review
gst.supp: add a suppression from libsoup (645 bytes, patch)
2015-03-26 12:53 UTC, Guillaume Desmottes
committed Details | Review
gst.supp: ignore leaks from libtool wrappers (779 bytes, patch)
2015-03-27 11:20 UTC, Guillaume Desmottes
accepted-commit_now Details | Review
gst.supp: add a TLS/Pango related trace (685 bytes, patch)
2015-03-30 09:03 UTC, Guillaume Desmottes
needs-work Details | Review

Description Guillaume Desmottes 2015-03-23 15:07:07 UTC
According to desrt, closures aren't very valgrind friendly because of pointer tricks.

bug #739850 is supposed to improve the situation but for now adding a few entries to gst.supp allows me to have 0 leak reported with gst-validate.
Comment 1 Guillaume Desmottes 2015-03-23 15:07:49 UTC
Created attachment 300140 [details] [review]
gst.supp: add some closure related entries

According to desrt, closures aren't very valgrind friendly because of pointer
tricks.

bgo#739850 is supposed to improve the situation but for now adding a few
entries to gst.supp allows me to have 0 leak reported with gst-validate.

Fix
Comment 2 Guillaume Desmottes 2015-03-26 10:38:35 UTC
Actually let's use this bug for all the gst.supp improvevements I'm doing while valgrinding gst-validate tests.
Comment 3 Guillaume Desmottes 2015-03-26 10:39:07 UTC
Created attachment 300345 [details] [review]
gst.supp: update gst-libav suppression

- Fix the function wildcard: 'fun:*' doesn't work, '...' does
- gst_ffmpegenc_register has been renamed to gst_ffmpegvidenc_register
- Remove redundant suppressions, they are a subset of the most generic one
Comment 4 Guillaume Desmottes 2015-03-26 12:53:24 UTC
Created attachment 300351 [details] [review]
gst.supp: add a suppression from libsoup

Needed for tests using http.
Comment 5 Guillaume Desmottes 2015-03-27 11:20:40 UTC
Created attachment 300431 [details] [review]
gst.supp: ignore leaks from libtool wrappers
Comment 6 Guillaume Desmottes 2015-03-30 09:03:27 UTC
Created attachment 300567 [details] [review]
gst.supp: add a TLS/Pango related trace
Comment 7 Nicolas Dufresne (ndufresne) 2015-03-30 20:23:36 UTC
Review of attachment 300431 [details] [review]:

This one looks suspicious, why would the parser leak something ?
Comment 8 Nicolas Dufresne (ndufresne) 2015-03-30 20:24:31 UTC
Review of attachment 300567 [details] [review]:

This is a real leak isn't it ?
Comment 9 Nicolas Dufresne (ndufresne) 2015-03-30 20:24:50 UTC
Review of attachment 300351 [details] [review]:

Looks good.
Comment 10 Nicolas Dufresne (ndufresne) 2015-03-30 20:25:22 UTC
Review of attachment 300345 [details] [review]:

Looks good.
Comment 11 Nicolas Dufresne (ndufresne) 2015-03-30 20:25:40 UTC
Review of attachment 300140 [details] [review]:

Looks good.
Comment 12 Guillaume Desmottes 2015-03-31 07:41:36 UTC
Review of attachment 300431 [details] [review]:

No idea, the wrapper is a shell script so I didn't bother digging more and just went for this easy workaround.
Comment 13 Guillaume Desmottes 2015-03-31 07:43:00 UTC
Review of attachment 300567 [details] [review]:

Not clear, "possible" leaks are always tricky. I experienced similar false positives deep in the pango stack previously so didn't investigate much atm.
Comment 14 Nicolas Dufresne (ndufresne) 2015-03-31 11:49:25 UTC
Review of attachment 300431 [details] [review]:

Ok, this one will need work. This is a leak in a lex parser we have written (which in in C). The comment speaks about libtool and then you speak about a shell parser. This need to be checked.
Comment 15 Nicolas Dufresne (ndufresne) 2015-04-02 22:24:12 UTC
Comment on attachment 300567 [details] [review]
gst.supp: add a TLS/Pango related trace

Does not apply anymore.
Comment 16 Nicolas Dufresne (ndufresne) 2015-04-02 22:25:13 UTC
Comment on attachment 300345 [details] [review]
gst.supp: update gst-libav suppression

Someone equivalence has been merged.
Comment 17 Nicolas Dufresne (ndufresne) 2015-04-02 22:25:38 UTC
Attachment 300140 [details] pushed as da5970f - gst.supp: add some closure related entries
Attachment 300351 [details] pushed as 274c71f - gst.supp: add a suppression from libsoup
Comment 18 Guillaume Desmottes 2015-04-08 12:20:56 UTC
(In reply to Nicolas Dufresne (stormer) from comment #14)
> Review of attachment 300431 [details] [review] [review]:
> 
> Ok, this one will need work. This is a leak in a lex parser we have written
> (which in in C). The comment speaks about libtool and then you speak about a
> shell parser. This need to be checked.

Which lex parser is that?

This is how the leak is reported by valgrind so I assumed it was in libtool's shell wrapper.

==11031== 20,976 (32 direct, 20,944 indirect) bytes in 1 blocks are definitely lost in loss record 893 of 893
==11031==    at 0x4A08BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==11031==    by 0x47199A: xmalloc (in /usr/bin/bash)
==11031==    by 0x43096B: make_if_command (in /usr/bin/bash)
==11031==    by 0x42C060: yyparse (in /usr/bin/bash)
==11031==    by 0x4229DA: parse_command (in /usr/bin/bash)
==11031==    by 0x422AA7: read_command (in /usr/bin/bash)
==11031==    by 0x422C88: reader_loop (in /usr/bin/bash)
==11031==    by 0x4215CD: main (in /usr/bin/bash)
Comment 19 Guillaume Desmottes 2015-04-13 10:18:26 UTC
(In reply to Guillaume Desmottes from comment #6)
> Created attachment 300567 [details] [review] [review]
> gst.supp: add a TLS/Pango related trace

For the record, this one can be reproduced using the "validate.file.compositor.simple.play_15s.synchronized" scenario.
Comment 20 Guillaume Desmottes 2015-04-13 15:32:11 UTC
(In reply to Guillaume Desmottes from comment #18)
> (In reply to Nicolas Dufresne (stormer) from comment #14)
> > Review of attachment 300431 [details] [review] [review] [review]:
> > 
> > Ok, this one will need work. This is a leak in a lex parser we have written
> > (which in in C). The comment speaks about libtool and then you speak about a
> > shell parser. This need to be checked.
> 
> Which lex parser is that?
> 
> This is how the leak is reported by valgrind so I assumed it was in
> libtool's shell wrapper.

After some more investigations, I can confirm that this leak is indeed because of libtool's wrapper. It only occurs the first time we launch gst-validate-media-check-1.0 when tools/.libs/lt-gst-validate-media-check-1.0 hasn't been generated yet. I guess we hit some specific path in the shell script raising this leak.
Comment 21 Nicolas Dufresne (ndufresne) 2015-04-22 17:13:13 UTC
Comment on attachment 300431 [details] [review]
gst.supp: ignore leaks from libtool wrappers

Good point, and with the entire stack trace it looks perfectly fine.
Comment 22 Guillaume Desmottes 2015-04-22 19:19:48 UTC
Comment on attachment 300431 [details] [review]
gst.supp: ignore leaks from libtool wrappers

Actually we don't need this any more. We are now using rpath when running from source in gst-validate so we don't have to bother about those wrappers.
Comment 23 Nicolas Dufresne (ndufresne) 2015-04-22 19:29:10 UTC
So remains the pango supp, maybe we should merge it, it's not our fault, neither it's our code after all. Anyone has any opinion ?
Comment 24 Guillaume Desmottes 2015-04-23 09:20:52 UTC
(In reply to Nicolas Dufresne (stormer) from comment #23)
> So remains the pango supp, maybe we should merge it, it's not our fault,
> neither it's our code after all. Anyone has any opinion ?

This one is actually fixed when using pixman master. Thibault and I decided to use a specific supp file for such known bugs (gstvalidate.supp).

So I think we're all good for now; closing.