Skip to content

Copy binary files for each target in FileSystemPublisher.initFiles() #63

Closed

Description

@kazupon

Summary

  • Goal: FileSystemPublisher.initFiles(files, binaryFiles) delivers binary files to every target and keeps the caller's buffers usable, like writeFile().
  • Current gap: initFiles() passes the same transfer list, which holds the caller's ArrayBuffers, to the postMessage() of every target.
    • The first call transfers the buffers, which detaches them.
    • With two or more targets, the next call therefore throws a DataCloneError.
    • With one target, the caller's buffers end up detached.
  • Effect:
    • With two targets, such as a Web Worker and a Service Worker, initFiles() throws after only the first target has received the files.
    • With any number of targets, the caller loses its ArrayBuffers.
  • Request: Copy each binary file for each target, as writeFile() does since #61.
  • Verification status: Reproduced in Node.js v24.21.0 with MessageChannel ports and the built package (see Details). Not checked in a browser. Transferring a detached ArrayBuffer throws a DataCloneError there as well.
  • Related: Found while implementing #42, which fixed the same problem in writeFile(). Vrowzer itself does not call initFiles(), but @vrowzer/fs/watcher exports it.

Details

Current implementation, verification, requested behavior, and acceptance criteria

Current implementation

Reviewed at ba60d7b.

  • initFiles() builds one transfer list from Object.values(binaryFiles) and passes it to broadcast(). broadcast() calls postMessage(message, transfer) on every target with that same list.
  • writeFile() already sends each target its own copy and transfers only the copy.
  • The JSDoc of FSInitMessage and of initFiles() says that the binary files are transferred.

Verification

I ran the following against packages/fs/dist/watcher/index.mjs, built from ba60d7b, with Node.js v24.21.0:

const first = new MessageChannel()
const second = new MessageChannel()
const publisher = createFileSystemPublisher([first.port1, second.port1])
const buffer = new Uint8Array([1, 2, 3]).buffer

publisher.initFiles(undefined, { '/a.bin': buffer })
// throws DataCloneError: Cannot transfer object of unsupported type.
buffer.byteLength // 0

const single = new MessageChannel()
const other = new Uint8Array([4, 5]).buffer
createFileSystemPublisher([single.port1]).initFiles(undefined, { '/b.bin': other })
other.byteLength // 0

The existing unit tests use mock targets, which do not detach transferred buffers. They also check initFiles() with one target only.

Requested behavior

  • Each target receives its own copy of every binary file, with the same bytes.
  • The caller's ArrayBuffers stay usable.
  • Text files are sent as before.

Acceptance criteria

  • With two targets, initFiles() delivers the text and binary files to both, and the binary files have the same bytes.
  • After initFiles() returns, the caller's ArrayBuffers keep their size and content.
  • The tests use targets that really transfer buffers, such as MessageChannel ports.
  • The JSDoc of FSInitMessage and initFiles() describes the copies.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions