Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit dd51ba3

Browse filesBrowse files
addaleaxaddaleax
committed
worker,fs: make FileHandle transferable
Allow passing `FileHandle` instances in the transfer list of a `.postMessage()` call. PR-URL: #33772 Backport-PR-URL: #33965 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 0d35eaa commit dd51ba3
Copy full SHA for dd51ba3
Expand file treeCollapse file tree

8 files changed

+235
-4
lines changed
Open diff view settings
Collapse file

‎doc/api/worker_threads.md‎

Copy file name to clipboardExpand all lines: doc/api/worker_threads.md
+10-2Lines changed: 10 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,10 @@ are part of the channel.
318318
### `port.postMessage(value[, transferList])`
319319
<!-- YAML
320320
added: v10.5.0
321+
changes:
322+
- version: REPLACEME
323+
pr-url: https://github.com/nodejs/node/pull/33772
324+
description: Added `FileHandle` to the list of transferable types.
321325
-->
322326

323327
* `value` {any}
@@ -335,7 +339,8 @@ In particular, the significant differences to `JSON` are:
335339
* `value` may contain typed arrays, both using `ArrayBuffer`s
336340
and `SharedArrayBuffer`s.
337341
* `value` may contain [`WebAssembly.Module`][] instances.
338-
* `value` may not contain native (C++-backed) objects other than `MessagePort`s.
342+
* `value` may not contain native (C++-backed) objects other than `MessagePort`s
343+
and [`FileHandle`][]s.
339344

340345
```js
341346
const { MessageChannel } = require('worker_threads');
@@ -349,7 +354,8 @@ circularData.foo = circularData;
349354
port2.postMessage(circularData);
350355
```
351356

352-
`transferList` may be a list of `ArrayBuffer` and `MessagePort` objects.
357+
`transferList` may be a list of [`ArrayBuffer`][], [`MessagePort`][] and
358+
[`FileHandle`][] objects.
353359
After transferring, they will not be usable on the sending side of the channel
354360
anymore (even if they are not contained in `value`). Unlike with
355361
[child processes][], transferring handles such as network sockets is currently
@@ -813,13 +819,15 @@ active handle in the event system. If the worker is already `unref()`ed calling
813819

814820
[`'close'` event]: #worker_threads_event_close
815821
[`'exit'` event]: #worker_threads_event_exit
822+
[`ArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer
816823
[`AsyncResource`]: async_hooks.html#async_hooks_class_asyncresource
817824
[`Buffer`]: buffer.html
818825
[`Buffer.allocUnsafe()`]: buffer.html#buffer_static_method_buffer_allocunsafe_size
819826
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: errors.html#errors_err_missing_message_port_in_transfer_list
820827
[`ERR_WORKER_NOT_RUNNING`]: errors.html#ERR_WORKER_NOT_RUNNING
821828
[`EventEmitter`]: events.html
822829
[`EventTarget`]: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget
830+
[`FileHandle`]: fs.html#fs_class_filehandle
823831
[`MessagePort`]: #worker_threads_class_messageport
824832
[`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
825833
[`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array
Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+26-2Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,17 @@ const { promisify } = require('internal/util');
5454
const kHandle = Symbol('kHandle');
5555
const kFd = Symbol('kFd');
5656
const { kUsePromises } = binding;
57+
const {
58+
JSTransferable, kDeserialize, kTransfer, kTransferList
59+
} = require('internal/worker/js_transferable');
5760

5861
const getDirectoryEntriesPromise = promisify(getDirents);
5962

60-
class FileHandle {
63+
class FileHandle extends JSTransferable {
6164
constructor(filehandle) {
65+
super();
6266
this[kHandle] = filehandle;
63-
this[kFd] = filehandle.fd;
67+
this[kFd] = filehandle ? filehandle.fd : -1;
6468
}
6569

6670
getAsyncId() {
@@ -131,6 +135,26 @@ class FileHandle {
131135
this[kFd] = -1;
132136
return this[kHandle].close();
133137
}
138+
139+
[kTransfer]() {
140+
const handle = this[kHandle];
141+
this[kFd] = -1;
142+
this[kHandle] = null;
143+
144+
return {
145+
data: { handle },
146+
deserializeInfo: 'internal/fs/promises:FileHandle'
147+
};
148+
}
149+
150+
[kTransferList]() {
151+
return [ this[kHandle] ];
152+
}
153+
154+
[kDeserialize]({ handle }) {
155+
this[kHandle] = handle;
156+
this[kFd] = handle.fd;
157+
}
134158
}
135159

136160
function validateFileHandle(handle) {
Collapse file

‎lib/internal/worker/js_transferable.js‎

Copy file name to clipboardExpand all lines: lib/internal/worker/js_transferable.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
'use strict';
2+
const { Error } = primordials;
23
const {
34
messaging_deserialize_symbol,
45
messaging_transfer_symbol,
@@ -17,6 +18,13 @@ function setup() {
1718
setDeserializerCreateObjectFunction((deserializeInfo) => {
1819
const [ module, ctor ] = deserializeInfo.split(':');
1920
const Ctor = require(module)[ctor];
21+
if (typeof Ctor !== 'function' ||
22+
!(Ctor.prototype instanceof JSTransferable)) {
23+
// Not one of the official errors because one should not be able to get
24+
// here without messing with Node.js internals.
25+
// eslint-disable-next-line no-restricted-syntax
26+
throw new Error(`Unknown deserialize spec ${deserializeInfo}`);
27+
}
2028
return new Ctor();
2129
});
2230
}
Collapse file

‎src/node_file.cc‎

Copy file name to clipboardExpand all lines: src/node_file.cc
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,40 @@ void FileHandle::MemoryInfo(MemoryTracker* tracker) const {
185185
tracker->TrackField("current_read", current_read_);
186186
}
187187

188+
FileHandle::TransferMode FileHandle::GetTransferMode() const {
189+
return reading_ || closing_ || closed_ ?
190+
TransferMode::kUntransferable : TransferMode::kTransferable;
191+
}
192+
193+
std::unique_ptr<worker::TransferData> FileHandle::TransferForMessaging() {
194+
CHECK_NE(GetTransferMode(), TransferMode::kUntransferable);
195+
auto ret = std::make_unique<TransferData>(fd_);
196+
closed_ = true;
197+
return std::move(ret);
198+
}
199+
200+
FileHandle::TransferData::TransferData(int fd) : fd_(fd) {}
201+
202+
FileHandle::TransferData::~TransferData() {
203+
if (fd_ > 0) {
204+
uv_fs_t close_req;
205+
CHECK_EQ(0, uv_fs_close(nullptr, &close_req, fd_, nullptr));
206+
uv_fs_req_cleanup(&close_req);
207+
}
208+
}
209+
210+
BaseObjectPtr<BaseObject> FileHandle::TransferData::Deserialize(
211+
Environment* env_,
212+
v8::Local<v8::Context> context,
213+
std::unique_ptr<worker::TransferData> self) {
214+
Environment* env = Environment::GetCurrent(context);
215+
if (env == nullptr) return {};
216+
217+
int fd = fd_;
218+
fd_ = -1;
219+
return BaseObjectPtr<BaseObject> { FileHandle::New(env, fd) };
220+
}
221+
188222
// Close the file descriptor if it hasn't already been closed. A process
189223
// warning will be emitted using a SetImmediate to avoid calling back to
190224
// JS during GC. If closing the fd fails at this point, a fatal exception
Collapse file

‎src/node_file.h‎

Copy file name to clipboardExpand all lines: src/node_file.h
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include "node.h"
77
#include "aliased_buffer.h"
8+
#include "node_messaging.h"
89
#include "stream_base.h"
910
#include <iostream>
1011

@@ -250,7 +251,28 @@ class FileHandle final : public AsyncWrap, public StreamBase {
250251
FileHandle(const FileHandle&&) = delete;
251252
FileHandle& operator=(const FileHandle&&) = delete;
252253

254+
TransferMode GetTransferMode() const override;
255+
std::unique_ptr<worker::TransferData> TransferForMessaging() override;
256+
253257
private:
258+
class TransferData : public worker::TransferData {
259+
public:
260+
explicit TransferData(int fd);
261+
~TransferData();
262+
263+
BaseObjectPtr<BaseObject> Deserialize(
264+
Environment* env,
265+
v8::Local<v8::Context> context,
266+
std::unique_ptr<worker::TransferData> self) override;
267+
268+
SET_NO_MEMORY_INFO()
269+
SET_MEMORY_INFO_NAME(FileHandleTransferData)
270+
SET_SELF_SIZE(TransferData)
271+
272+
private:
273+
int fd_;
274+
};
275+
254276
FileHandle(Environment* env, v8::Local<v8::Object> obj, int fd);
255277

256278
// Synchronous close that emits a warning
Collapse file

‎test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js‎

Copy file name to clipboard
+33Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const fs = require('fs').promises;
5+
const { MessageChannel } = require('worker_threads');
6+
const { once } = require('events');
7+
8+
// Test that overriding the internal kTransfer method of a JSTransferable does
9+
// not enable loading arbitrary code from internal Node.js core modules.
10+
11+
(async function() {
12+
const fh = await fs.open(__filename);
13+
assert.strictEqual(fh.constructor.name, 'FileHandle');
14+
15+
const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh))
16+
.filter((symbol) => symbol.description === 'messaging_transfer_symbol')[0];
17+
assert.strictEqual(typeof kTransfer, 'symbol');
18+
fh[kTransfer] = () => {
19+
return {
20+
data: '✨',
21+
deserializeInfo: 'net:Socket'
22+
};
23+
};
24+
25+
const { port1, port2 } = new MessageChannel();
26+
port1.postMessage(fh, [ fh ]);
27+
port2.on('message', common.mustNotCall());
28+
29+
const [ exception ] = await once(process, 'uncaughtException');
30+
31+
assert.strictEqual(exception.message, 'Unknown deserialize spec net:Socket');
32+
port2.close();
33+
})().then(common.mustCall());
Collapse file

‎test/parallel/test-worker-message-port-transfer-fake-js-transferable.js‎

Copy file name to clipboard
+37Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const fs = require('fs').promises;
5+
const { MessageChannel } = require('worker_threads');
6+
const { once } = require('events');
7+
8+
// Test that overriding the internal kTransfer method of a JSTransferable does
9+
// not enable loading arbitrary code from the disk.
10+
11+
module.exports = {
12+
NotARealClass: common.mustNotCall()
13+
};
14+
15+
(async function() {
16+
const fh = await fs.open(__filename);
17+
assert.strictEqual(fh.constructor.name, 'FileHandle');
18+
19+
const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh))
20+
.filter((symbol) => symbol.description === 'messaging_transfer_symbol')[0];
21+
assert.strictEqual(typeof kTransfer, 'symbol');
22+
fh[kTransfer] = () => {
23+
return {
24+
data: '✨',
25+
deserializeInfo: `${__filename}:NotARealClass`
26+
};
27+
};
28+
29+
const { port1, port2 } = new MessageChannel();
30+
port1.postMessage(fh, [ fh ]);
31+
port2.on('message', common.mustNotCall());
32+
33+
const [ exception ] = await once(process, 'uncaughtException');
34+
35+
assert.match(exception.message, /Missing internal module/);
36+
port2.close();
37+
})().then(common.mustCall());
Collapse file

‎test/parallel/test-worker-message-port-transfer-filehandle.js‎

Copy file name to clipboard
+65Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const fs = require('fs').promises;
5+
const vm = require('vm');
6+
const { MessageChannel, moveMessagePortToContext } = require('worker_threads');
7+
const { once } = require('events');
8+
9+
(async function() {
10+
const fh = await fs.open(__filename);
11+
12+
const { port1, port2 } = new MessageChannel();
13+
14+
assert.throws(() => {
15+
port1.postMessage(fh);
16+
}, {
17+
// See the TODO about error code in node_messaging.cc.
18+
code: 'ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST'
19+
});
20+
21+
// Check that transferring FileHandle instances works.
22+
assert.notStrictEqual(fh.fd, -1);
23+
port1.postMessage(fh, [ fh ]);
24+
assert.strictEqual(fh.fd, -1);
25+
26+
const [ fh2 ] = await once(port2, 'message');
27+
assert.strictEqual(Object.getPrototypeOf(fh2), Object.getPrototypeOf(fh));
28+
29+
assert.deepStrictEqual(await fh2.readFile(), await fs.readFile(__filename));
30+
await fh2.close();
31+
32+
assert.rejects(() => fh.readFile(), { code: 'EBADF' });
33+
})().then(common.mustCall());
34+
35+
(async function() {
36+
// Check that there is no crash if the message is never read.
37+
const fh = await fs.open(__filename);
38+
39+
const { port1 } = new MessageChannel();
40+
41+
assert.notStrictEqual(fh.fd, -1);
42+
port1.postMessage(fh, [ fh ]);
43+
assert.strictEqual(fh.fd, -1);
44+
})().then(common.mustCall());
45+
46+
(async function() {
47+
// Check that in the case of a context mismatch the message is discarded.
48+
const fh = await fs.open(__filename);
49+
50+
const { port1, port2 } = new MessageChannel();
51+
52+
const ctx = vm.createContext();
53+
const port2moved = moveMessagePortToContext(port2, ctx);
54+
port2moved.onmessage = common.mustCall((msgEvent) => {
55+
assert.strictEqual(msgEvent.data, 'second message');
56+
port1.close();
57+
});
58+
port2moved.start();
59+
60+
assert.notStrictEqual(fh.fd, -1);
61+
port1.postMessage(fh, [ fh ]);
62+
assert.strictEqual(fh.fd, -1);
63+
64+
port1.postMessage('second message');
65+
})().then(common.mustCall());

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.