Closed
Bug 1055842
Opened 10 years ago
Closed 10 years ago
js1_8_5/extensions/clone-transferables.js crashes starting with c1ea647e6a70
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: Waldo, Assigned: sfink)
References
Details
Attachments
(2 files)
1.37 KB,
patch
|
Details | Diff | Splinter Review | |
6.11 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
This test consistently crashes after the landing of bug 1037358. I briefly tried debugging, but my debugger seems to say it crashes inside malloc internals (!) and won't let me backtrace, step, or anything else, I think maybe because some sort of lock is held. Filing now in case someone else sees the obvious issue here.
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → sphink
Assignee | ||
Comment 1•10 years ago
|
||
Started looking into this, but got distracted by a topcrash. What I know so far is that it's a result of unmapping a pointer. Perhaps it was a malloced pointer? Anyway, will keep looking.
Assignee | ||
Comment 2•10 years ago
|
||
The problem is that when stealing the contents of a mapped array buffer, the contents returned might be mapped or malloced. Specifically what is happening is that an asmjs arraybuffer, which is mapped, is transferred via structured clone. Since asmjs buffers cannot simply be neutered/detached, they are instead duplicated and left alone, returning freshly-malloced data. But because isMappedArrayBuffer() returned true, the structured clone code erroneously assumes that the stolen pointer is also to mapped data.
Previously, isMappedArrayBuffer() returned false for asmjs buffers, even though they're mapped, because there was a separate bit for "asmjs mapped" vs "other mapped".
Neither the old nor the new approach is really right. The structured clone code should not be guessing what type of memory JS_StealArrayBufferContents is returning. I'll look into making it self-describing.
I guess this hasn't been widely noticed because asm.js buffers aren't commonly transferred?
Comment 3•10 years ago
|
||
sfink's suggested workaround, from irc.
Assignee | ||
Comment 4•10 years ago
|
||
JS_StealTransferableContents is still footgunny, but I'd like to get a targeted fix in before thinking about expanding the public API.
Attachment #8480997 -
Flags: review?(jwalden+bmo)
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8480997 [details] [diff] [review]
Communicate ownership of stolen ArrayBuffer contents
Review of attachment 8480997 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/vm/ArrayBufferObject.cpp
@@ +786,1 @@
> } else {
Pre-existing else after return here.
@@ +1167,5 @@
> return nullptr;
> }
>
> Rooted<ArrayBufferObject*> buffer(cx, &obj->as<ArrayBufferObject>());
> + return ArrayBufferObject::stealContents(cx, buffer).data();
Maybe add some sort of warning printf or something if the contents kind is "screwball"?
Attachment #8480997 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Comment 7•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•