Closed
Bug 1246157
Opened 9 years ago
Closed 9 years ago
[Static Analysis][Resource Leak] In functions ModuleObject::createNamespace and ModuleObject::create
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: andi, Assigned: andi)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: CID 1345749, CID 1345748)
Attachments
(1 file, 1 obsolete file)
1.61 KB,
patch
|
Details | Diff | Splinter Review |
The Static Analysis tool Coverity added that memory is leaked on variable bindings if bindings->init() fails. The behavior is identical in both function so i will stick only to createNamespace:
>> IndirectBindingMap* bindings = zone->new_<IndirectBindingMap>(zone);
>> if (!bindings || !bindings->init()) {
>> ReportOutOfMemory(cx);
>> return nullptr;
>> }
bindings gets memory allocated by by path pod_malloc<uint8_t>()->maybe_pod_malloc<T>()->js_pod_malloc<T>()->js_malloc()->malloc()
If allocation is successful and bindings->init() fails, most likely due to oom when the table for the hash is created:
>> table = createTable(*this, newCapacity);
>> if (!table)
>> return false;
There will be a memory leak since since the memory previously allocated is not freed
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8716302 -
Flags: review?(jorendorff)
Comment 2•9 years ago
|
||
Comment on attachment 8716302 [details] [diff] [review]
avoid memory leak when bindings->init fails
Review of attachment 8716302 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good, but for an even simpler change I think you can just add |js_delete(bindings);| to the existing if blocks, since deleting a null pointer does nothing.
Thanks for fixing this!
Attachment #8716302 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8716302 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Comment 5•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•