Closed Bug 1246157 Opened 4 years ago Closed 4 years ago

[Static Analysis][Resource Leak] In functions ModuleObject::createNamespace and ModuleObject::create

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

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)

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
Attachment #8716302 - Flags: review?(jorendorff)
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+
Attached patch Bug 1246157.diffSplinter Review
Attachment #8716302 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5253aba43949
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.