sipcc leaks like a sieve

RESOLVED FIXED in mozilla34

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: bwc, Assigned: bwc)

Tracking

(Blocks: 1 bug)

Trunk
mozilla34
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 8475552 [details]
LSAN output from SignalingTest.FullCall

I'm wrapping up my thread flattening patches, and tried out an LSAN run to see if I introduced any leaks.

You know what it's like opening that tupperware container that has been buried at the back of your fridge for longer than you can remember? You know, that "I should not have opened that." moment?

Yeah...
(Assignee)

Comment 1

5 years ago
I think I will mark this as dependent on bug 991037 because there is at least one place where plugging the leak is much easier if we can assume that there are no async dispatches and such.
(Assignee)

Updated

5 years ago
Depends on: 991037
(Assignee)

Comment 2

5 years ago
Created attachment 8477005 [details] [diff] [review]
Plug bunches of leaks that signaling_unittest hits, mostly in sipcc.

What a mess.
(Assignee)

Updated

5 years ago
Assignee: nobody → docfaraday
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #8477005 - Flags: review?(martin.thomson)
(Assignee)

Comment 3

5 years ago
Created attachment 8477710 [details] [diff] [review]
Part 2: Remove the suppressions for these leaks.

Update the suppressions file.
Fantastic!
Whiteboard: [MemShrink]
Blocks: 979928
Attachment #8477005 - Flags: review?(martin.thomson) → review+
(Assignee)

Comment 6

5 years ago
Who should I ask to review the suppressions file?
Flags: needinfo?(continuation)
Comment on attachment 8477710 [details] [diff] [review]
Part 2: Remove the suppressions for these leaks.

I can just review it.  You are just removing suppressions so there shouldn't be a problem. :)
Attachment #8477710 - Flags: review+
Flags: needinfo?(continuation)
(Assignee)

Updated

5 years ago
No longer blocks: 1057804
(Assignee)

Comment 8

5 years ago
Created attachment 8479383 [details] [diff] [review]
Part 2: Remove the suppressions for these leaks

Unrot.
(Assignee)

Updated

5 years ago
Attachment #8477710 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
Comment on attachment 8479383 [details] [diff] [review]
Part 2: Remove the suppressions for these leaks

Review of attachment 8479383 [details] [diff] [review]:
-----------------------------------------------------------------

Carry forward r=mccr8
Attachment #8479383 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
seems this patch failed to apply 

patching file build/sanitizers/lsan_suppressions.txt
Hunk #1 FAILED at 27
1 out of 1 hunks FAILED -- saving rejects to file build/sanitizers/lsan_suppressions.txt.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh remove_suppressions

Byron: could you take a look and maybe rebase, thanks!
Flags: needinfo?(docfaraday)
(In reply to Carsten Book [:Tomcat] from comment #11)
> seems this patch failed to apply 
> 
> patching file build/sanitizers/lsan_suppressions.txt
> Hunk #1 FAILED at 27
> 1 out of 1 hunks FAILED -- saving rejects to file
> build/sanitizers/lsan_suppressions.txt.rej
> patch failed, unable to continue (try -v)
> patch failed, rejects left in working dir
> errors during apply, please fix and refresh remove_suppressions
> 
> Byron: could you take a look and maybe rebase, thanks!

note this was for part 2. Part 1 was fine and landed as https://hg.mozilla.org/integration/mozilla-inbound/rev/103cf4c29d7f
Keywords: leave-open
I've rebased this and will push it in a bit.
Flags: needinfo?(docfaraday)
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/5637f948d184
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1041113
You need to log in before you can comment on or make changes to this bug.