Closed Bug 1243754 Opened 5 years ago Closed 5 years ago

TEST-UNEXPECTED-FAIL | valgrind-test | 4,096 bytes in 1 blocks are definitely lost at malloc / js_malloc / new_ / getOrCreateChunk

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: h4writer, Assigned: h4writer)

References

Details

Attachments

(1 file, 1 obsolete file)

Fallout from landing of shard stubs in bug 1241087
Blocks: 1241087
Attached patch Free the stubspace (obsolete) — Splinter Review
Small patch. I failed to account for the different way BaselineScript and IonScript are destroyed. As a result the fallbackspace wasn't free'ed. Manually freeing here.
Assignee: nobody → hv1989
Attachment #8713534 - Flags: review?(jdemooij)
Comment on attachment 8713534 [details] [diff] [review]
Free the stubspace

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

::: js/src/jit/Ion.cpp
@@ +1280,5 @@
>  {
>      script->unlinkFromRuntime(fop);
>      // Frees the potential event we have set.
>      script->traceLoggerScriptEvent_ = TraceLoggerEvent();
> +    script->fallbackStubSpace_.free();

Instead of these last two lines, can we use fop->delete_(script) below, instead of free_?
Comment on attachment 8713534 [details] [diff] [review]
Free the stubspace

Clearing per comment 2.
Flags: needinfo?(hv1989)
Attachment #8713534 - Flags: review?(jdemooij)
Indeed a little bit more elegant.
Attachment #8713534 - Attachment is obsolete: true
Flags: needinfo?(hv1989)
Attachment #8715774 - Flags: review?(jdemooij)
Comment on attachment 8715774 [details] [diff] [review]
Delete the IonScript

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

Thanks!
Attachment #8715774 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/374422755fcc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.