• Anna Henningsen's avatar
    [api] Add possibility for BackingStore to keep Allocator alive · 6b0a9535
    Anna Henningsen authored
    Add an `array_buffer_allocator_shared` field to the
    `Isolate::CreateParams` struct that allows embedders to share
    ownership of the ArrayBuffer::Allocator with V8, and which in
    particular means that when this method is used that the
    BackingStore deleter will not perform an use-after-free access to the
    Allocator under certain circumstances.
    
    For Background:
    
    tl;dr: This is necessary for Node.js to perform the transition to
    V8 7.9, because of the way that ArrayBuffer::Allocators and their
    lifetimes currently work there.
    
    In Node.js, each Worker thread has its own ArrayBuffer::Allocator.
    Changing that would currently be impractical, as each allocator
    depends on per-Isolate state. However, now that backing stores
    are managed globally and keep a pointer to the original
    ArrayBuffer::Allocator, this means that when transferring an
    ArrayBuffer (e.g. from one Worker to another through postMessage()),
    the original Allocator has to be kept alive until the ArrayBuffer
    no longer exists in the receiving Isolate (or until that Isolate
    is disposed). See [1] for an example Node.js test that fails with
    V8 7.9.
    
    This problem also existed for SharedArrayBuffers, where Node.js
    was broken by V8 earlier for the same reasons (see [2] for the bug
    report on that and [3] for the resolution in Node.js).
    For SharedArrayBuffers, we already had extensive tracking logic,
    so adding a shared_ptr to keep alive the ArrayBuffer::Allocator
    was not a significant amount of work. However, the mechanism for
    transferring non-shared ArrayBuffers is quite different, and
    it seems both easier for us and better for V8 from an API standpoint
    to keep the Allocator alive from where it is being referenced.
    
    By sharing memory with the custom deleter function/data pair,
    this comes at no memory overhead.
    
    [1]: https://github.com/nodejs/node/pull/30044
    [2]: https://github.com/nodejs/node-v8/issues/115
    [3]: https://github.com/nodejs/node/pull/29637
    
    Bug: v8:9380
    Change-Id: Ibc2c4fb6341b53653cbd637bd8cb3d4ac43809c7
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1874347
    Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
    Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#64542}
    6b0a9535
backing-store.h 9.59 KB