Last Comment Bug 777083 - ScriptSource::createFromSource leaks when ownSource is true
: ScriptSource::createFromSource leaks when ownSource is true
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-24 14:39 PDT by :Benjamin Peterson
Modified: 2012-07-25 08:10 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
plug leak (1.35 KB, patch)
2012-07-24 14:39 PDT, :Benjamin Peterson
luke: review+
Details | Diff | Splinter Review

Description :Benjamin Peterson 2012-07-24 14:39:36 PDT
Created attachment 645510 [details] [diff] [review]
plug leak

When ownSource is true, we simply set data.source to the source passed to createFromSource. However, we also unconditionally allocate a buffer for the compressed string, which we never use with ownSource. This means we leak a entire JS file anytime anyone calls chrome toSource. Oops.
Comment 1 Luke Wagner [:luke] 2012-07-24 15:13:40 PDT
I see two callers of createFromSource, the one that passes 'true' and one that passes 'false', but then unconditionally frees 'source'.  Could createFromSource always take ownership (even when it fails), allowing us to remove the parameter?
Comment 2 :Benjamin Peterson 2012-07-24 15:17:57 PDT
(In reply to Luke Wagner [:luke] from comment #1)
> I see two callers of createFromSource, the one that passes 'true' and one
> that passes 'false', but then unconditionally frees 'source'.  Could
> createFromSource always take ownership (even when it fails), allowing us to
> remove the parameter?

The main usage is in frontend/BytecodeCompiler.cpp. In those cases, we really do need to copy. The GlobalObject.cpp case is a weird one written before the ownSource parameter. It could use that now, actually.
Comment 4 Ed Morley [:emorley] 2012-07-25 08:10:37 PDT
https://hg.mozilla.org/mozilla-central/rev/a338900cf0af

Note You need to log in before you can comment on or make changes to this bug.