FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

worker,fs: make FileHandle transferable · nodejs/node@5b1fd10 · GitHub

/ node Public

Commit 5b1fd10

Browse files
addaleax
authored and
codebytere
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 9692208 commit 5b1fd10

8 files changed

Lines changed: 235 additions & 4 deletions

‎doc/api/worker_threads.md‎

Lines changed: 10 additions & 2 deletions
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
@@ -810,13 +816,15 @@ active handle in the event system. If the worker is already `unref()`ed calling
810816

811817
[`'close'` event]: #worker_threads_event_close
812818
[`'exit'` event]: #worker_threads_event_exit
819+
[`ArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer
813820
[`AsyncResource`]: async_hooks.html#async_hooks_class_asyncresource
814821
[`Buffer`]: buffer.html
815822
[`Buffer.allocUnsafe()`]: buffer.html#buffer_class_method_buffer_allocunsafe_size
816823
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: errors.html#errors_err_missing_message_port_in_transfer_list
817824
[`ERR_WORKER_NOT_RUNNING`]: errors.html#ERR_WORKER_NOT_RUNNING
818825
[`EventEmitter`]: events.html
819826
[`EventTarget`]: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget
827+
[`FileHandle`]: fs.html#fs_class_filehandle
820828
[`MessagePort`]: #worker_threads_class_messageport
821829
[`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
822830
[`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array

‎lib/internal/fs/promises.js‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,17 @@ const { promisify } = require('internal/util');
6565
const kHandle = Symbol('kHandle');
6666
const kFd = Symbol('kFd');
6767
const { kUsePromises } = binding;
68+
const {
69+
JSTransferable, kDeserialize, kTransfer, kTransferList
70+
} = require('internal/worker/js_transferable');
6871

6972
const getDirectoryEntriesPromise = promisify(getDirents);
7073

71-
class FileHandle {
74+
class FileHandle extends JSTransferable {
7275
constructor(filehandle) {
76+
super();
7377
this[kHandle] = filehandle;
74-
this[kFd] = filehandle.fd;
78+
this[kFd] = filehandle ? filehandle.fd : -1;
7579
}
7680

7781
getAsyncId() {
@@ -142,6 +146,26 @@ class FileHandle {
142146
this[kFd] = -1;
143147
return this[kHandle].close();
144148
}
149+
150+
[kTransfer]() {
151+
const handle = this[kHandle];
152+
this[kFd] = -1;
153+
this[kHandle] = null;
154+
155+
return {
156+
data: { handle },
157+
deserializeInfo: 'internal/fs/promises:FileHandle'
158+
};
159+
}
160+
161+
[kTransferList]() {
162+
return [ this[kHandle] ];
163+
}
164+
165+
[kDeserialize]({ handle }) {
166+
this[kHandle] = handle;
167+
this[kFd] = handle.fd;
168+
}
145169
}
146170

147171
function validateFileHandle(handle) {

‎lib/internal/worker/js_transferable.js‎

Lines 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
}

‎src/node_file.cc‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,40 @@ void FileHandle::MemoryInfo(MemoryTracker* tracker) const {
190190
tracker->TrackField("current_read", current_read_);
191191
}
192192

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

‎src/node_file.h‎

Lines 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

@@ -273,7 +274,28 @@ class FileHandle final : public AsyncWrap, public StreamBase {
273274
FileHandle(const FileHandle&&) = delete;
274275
FileHandle& operator=(const FileHandle&&) = delete;
275276

277+
TransferMode GetTransferMode() const override;
278+
std::unique_ptr<worker::TransferData> TransferForMessaging() override;
279+
276280
private:
281+
class TransferData : public worker::TransferData {
282+
public:
283+
explicit TransferData(int fd);
284+
~TransferData();
285+
286+
BaseObjectPtr<BaseObject> Deserialize(
287+
Environment* env,
288+
v8::Local<v8::Context> context,
289+
std::unique_ptr<worker::TransferData> self) override;
290+
291+
SET_NO_MEMORY_INFO()
292+
SET_MEMORY_INFO_NAME(FileHandleTransferData)
293+
SET_SELF_SIZE(TransferData)
294+
295+
private:
296+
int fd_;
297+
};
298+
277299
FileHandle(BindingData* binding_data, v8::Local<v8::Object> obj, int fd);
278300

279301
// Synchronous close that emits a warning

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

Lines 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());

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

Lines 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());

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

Lines 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)
Back | FazBrowse Home | New Git URL