Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/nodejs/node.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAnna Henningsen <anna@addaleax.net>2020-06-06 17:27:30 +0300
committerAnna Henningsen <anna@addaleax.net>2020-06-14 15:53:40 +0300
commit8641d94189398063b20d5e38549bfd8f023af2d6 (patch)
treeecb18fb2e97803c51b6b5105361a3e4fb0a5904d
parent9129cf21ab51432675521ea158191ae3f866cafb (diff)
worker,fs: make FileHandle transferable
Allow passing `FileHandle` instances in the transfer list of a `.postMessage()` call. PR-URL: https://github.com/nodejs/node/pull/33772 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
-rw-r--r--doc/api/worker_threads.md12
-rw-r--r--lib/internal/fs/promises.js28
-rw-r--r--lib/internal/worker/js_transferable.js8
-rw-r--r--src/node_file.cc34
-rw-r--r--src/node_file.h22
-rw-r--r--test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js33
-rw-r--r--test/parallel/test-worker-message-port-transfer-fake-js-transferable.js37
-rw-r--r--test/parallel/test-worker-message-port-transfer-filehandle.js65
8 files changed, 235 insertions, 4 deletions
diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md
index d4fd2f5179a..9a893a40972 100644
--- a/doc/api/worker_threads.md
+++ b/doc/api/worker_threads.md
@@ -318,6 +318,10 @@ are part of the channel.
### `port.postMessage(value[, transferList])`
<!-- YAML
added: v10.5.0
+changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/33772
+ description: Added `FileHandle` to the list of transferable types.
-->
* `value` {any}
@@ -335,7 +339,8 @@ In particular, the significant differences to `JSON` are:
* `value` may contain typed arrays, both using `ArrayBuffer`s
and `SharedArrayBuffer`s.
* `value` may contain [`WebAssembly.Module`][] instances.
-* `value` may not contain native (C++-backed) objects other than `MessagePort`s.
+* `value` may not contain native (C++-backed) objects other than `MessagePort`s
+ and [`FileHandle`][]s.
```js
const { MessageChannel } = require('worker_threads');
@@ -349,7 +354,8 @@ circularData.foo = circularData;
port2.postMessage(circularData);
```
-`transferList` may be a list of `ArrayBuffer` and `MessagePort` objects.
+`transferList` may be a list of [`ArrayBuffer`][], [`MessagePort`][] and
+[`FileHandle`][] objects.
After transferring, they will not be usable on the sending side of the channel
anymore (even if they are not contained in `value`). Unlike with
[child processes][], transferring handles such as network sockets is currently
@@ -816,6 +822,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`'close'` event]: #worker_threads_event_close
[`'exit'` event]: #worker_threads_event_exit
+[`ArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer
[`AsyncResource`]: async_hooks.html#async_hooks_class_asyncresource
[`Buffer`]: buffer.html
[`Buffer.allocUnsafe()`]: buffer.html#buffer_class_method_buffer_allocunsafe_size
@@ -823,6 +830,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`ERR_WORKER_NOT_RUNNING`]: errors.html#ERR_WORKER_NOT_RUNNING
[`EventEmitter`]: events.html
[`EventTarget`]: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget
+[`FileHandle`]: fs.html#fs_class_filehandle
[`MessagePort`]: #worker_threads_class_messageport
[`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
[`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 05d82991552..717b26e59fa 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -65,13 +65,17 @@ const { promisify } = require('internal/util');
const kHandle = Symbol('kHandle');
const kFd = Symbol('kFd');
const { kUsePromises } = binding;
+const {
+ JSTransferable, kDeserialize, kTransfer, kTransferList
+} = require('internal/worker/js_transferable');
const getDirectoryEntriesPromise = promisify(getDirents);
-class FileHandle {
+class FileHandle extends JSTransferable {
constructor(filehandle) {
+ super();
this[kHandle] = filehandle;
- this[kFd] = filehandle.fd;
+ this[kFd] = filehandle ? filehandle.fd : -1;
}
getAsyncId() {
@@ -142,6 +146,26 @@ class FileHandle {
this[kFd] = -1;
return this[kHandle].close();
}
+
+ [kTransfer]() {
+ const handle = this[kHandle];
+ this[kFd] = -1;
+ this[kHandle] = null;
+
+ return {
+ data: { handle },
+ deserializeInfo: 'internal/fs/promises:FileHandle'
+ };
+ }
+
+ [kTransferList]() {
+ return [ this[kHandle] ];
+ }
+
+ [kDeserialize]({ handle }) {
+ this[kHandle] = handle;
+ this[kFd] = handle.fd;
+ }
}
function validateFileHandle(handle) {
diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js
index 41f1e6ff72a..707fd03f2f6 100644
--- a/lib/internal/worker/js_transferable.js
+++ b/lib/internal/worker/js_transferable.js
@@ -1,4 +1,5 @@
'use strict';
+const { Error } = primordials;
const {
messaging_deserialize_symbol,
messaging_transfer_symbol,
@@ -17,6 +18,13 @@ function setup() {
setDeserializerCreateObjectFunction((deserializeInfo) => {
const [ module, ctor ] = deserializeInfo.split(':');
const Ctor = require(module)[ctor];
+ if (typeof Ctor !== 'function' ||
+ !(Ctor.prototype instanceof JSTransferable)) {
+ // Not one of the official errors because one should not be able to get
+ // here without messing with Node.js internals.
+ // eslint-disable-next-line no-restricted-syntax
+ throw new Error(`Unknown deserialize spec ${deserializeInfo}`);
+ }
return new Ctor();
});
}
diff --git a/src/node_file.cc b/src/node_file.cc
index c2ed56f43fb..9ed2d2960cf 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -190,6 +190,40 @@ void FileHandle::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("current_read", current_read_);
}
+FileHandle::TransferMode FileHandle::GetTransferMode() const {
+ return reading_ || closing_ || closed_ ?
+ TransferMode::kUntransferable : TransferMode::kTransferable;
+}
+
+std::unique_ptr<worker::TransferData> FileHandle::TransferForMessaging() {
+ CHECK_NE(GetTransferMode(), TransferMode::kUntransferable);
+ auto ret = std::make_unique<TransferData>(fd_);
+ closed_ = true;
+ return ret;
+}
+
+FileHandle::TransferData::TransferData(int fd) : fd_(fd) {}
+
+FileHandle::TransferData::~TransferData() {
+ if (fd_ > 0) {
+ uv_fs_t close_req;
+ CHECK_EQ(0, uv_fs_close(nullptr, &close_req, fd_, nullptr));
+ uv_fs_req_cleanup(&close_req);
+ }
+}
+
+BaseObjectPtr<BaseObject> FileHandle::TransferData::Deserialize(
+ Environment* env,
+ v8::Local<v8::Context> context,
+ std::unique_ptr<worker::TransferData> self) {
+ BindingData* bd = Environment::GetBindingData<BindingData>(context);
+ if (bd == nullptr) return {};
+
+ int fd = fd_;
+ fd_ = -1;
+ return BaseObjectPtr<BaseObject> { FileHandle::New(bd, fd) };
+}
+
// Close the file descriptor if it hasn't already been closed. A process
// warning will be emitted using a SetImmediate to avoid calling back to
// JS during GC. If closing the fd fails at this point, a fatal exception
diff --git a/src/node_file.h b/src/node_file.h
index ad493bd944c..fd17fc99d52 100644
--- a/src/node_file.h
+++ b/src/node_file.h
@@ -5,6 +5,7 @@
#include "node.h"
#include "aliased_buffer.h"
+#include "node_messaging.h"
#include "stream_base.h"
#include <iostream>
@@ -273,7 +274,28 @@ class FileHandle final : public AsyncWrap, public StreamBase {
FileHandle(const FileHandle&&) = delete;
FileHandle& operator=(const FileHandle&&) = delete;
+ TransferMode GetTransferMode() const override;
+ std::unique_ptr<worker::TransferData> TransferForMessaging() override;
+
private:
+ class TransferData : public worker::TransferData {
+ public:
+ explicit TransferData(int fd);
+ ~TransferData();
+
+ BaseObjectPtr<BaseObject> Deserialize(
+ Environment* env,
+ v8::Local<v8::Context> context,
+ std::unique_ptr<worker::TransferData> self) override;
+
+ SET_NO_MEMORY_INFO()
+ SET_MEMORY_INFO_NAME(FileHandleTransferData)
+ SET_SELF_SIZE(TransferData)
+
+ private:
+ int fd_;
+ };
+
FileHandle(BindingData* binding_data, v8::Local<v8::Object> obj, int fd);
// Synchronous close that emits a warning
diff --git a/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js b/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js
new file mode 100644
index 00000000000..10a50494338
--- /dev/null
+++ b/test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js
@@ -0,0 +1,33 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs').promises;
+const { MessageChannel } = require('worker_threads');
+const { once } = require('events');
+
+// Test that overriding the internal kTransfer method of a JSTransferable does
+// not enable loading arbitrary code from internal Node.js core modules.
+
+(async function() {
+ const fh = await fs.open(__filename);
+ assert.strictEqual(fh.constructor.name, 'FileHandle');
+
+ const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh))
+ .filter((symbol) => symbol.description === 'messaging_transfer_symbol')[0];
+ assert.strictEqual(typeof kTransfer, 'symbol');
+ fh[kTransfer] = () => {
+ return {
+ data: '✨',
+ deserializeInfo: 'net:Socket'
+ };
+ };
+
+ const { port1, port2 } = new MessageChannel();
+ port1.postMessage(fh, [ fh ]);
+ port2.on('message', common.mustNotCall());
+
+ const [ exception ] = await once(process, 'uncaughtException');
+
+ assert.strictEqual(exception.message, 'Unknown deserialize spec net:Socket');
+ port2.close();
+})().then(common.mustCall());
diff --git a/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js b/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js
new file mode 100644
index 00000000000..924d850a5d1
--- /dev/null
+++ b/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js
@@ -0,0 +1,37 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs').promises;
+const { MessageChannel } = require('worker_threads');
+const { once } = require('events');
+
+// Test that overriding the internal kTransfer method of a JSTransferable does
+// not enable loading arbitrary code from the disk.
+
+module.exports = {
+ NotARealClass: common.mustNotCall()
+};
+
+(async function() {
+ const fh = await fs.open(__filename);
+ assert.strictEqual(fh.constructor.name, 'FileHandle');
+
+ const kTransfer = Object.getOwnPropertySymbols(Object.getPrototypeOf(fh))
+ .filter((symbol) => symbol.description === 'messaging_transfer_symbol')[0];
+ assert.strictEqual(typeof kTransfer, 'symbol');
+ fh[kTransfer] = () => {
+ return {
+ data: '✨',
+ deserializeInfo: `${__filename}:NotARealClass`
+ };
+ };
+
+ const { port1, port2 } = new MessageChannel();
+ port1.postMessage(fh, [ fh ]);
+ port2.on('message', common.mustNotCall());
+
+ const [ exception ] = await once(process, 'uncaughtException');
+
+ assert.match(exception.message, /Missing internal module/);
+ port2.close();
+})().then(common.mustCall());
diff --git a/test/parallel/test-worker-message-port-transfer-filehandle.js b/test/parallel/test-worker-message-port-transfer-filehandle.js
new file mode 100644
index 00000000000..3765fca865e
--- /dev/null
+++ b/test/parallel/test-worker-message-port-transfer-filehandle.js
@@ -0,0 +1,65 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs').promises;
+const vm = require('vm');
+const { MessageChannel, moveMessagePortToContext } = require('worker_threads');
+const { once } = require('events');
+
+(async function() {
+ const fh = await fs.open(__filename);
+
+ const { port1, port2 } = new MessageChannel();
+
+ assert.throws(() => {
+ port1.postMessage(fh);
+ }, {
+ // See the TODO about error code in node_messaging.cc.
+ code: 'ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST'
+ });
+
+ // Check that transferring FileHandle instances works.
+ assert.notStrictEqual(fh.fd, -1);
+ port1.postMessage(fh, [ fh ]);
+ assert.strictEqual(fh.fd, -1);
+
+ const [ fh2 ] = await once(port2, 'message');
+ assert.strictEqual(Object.getPrototypeOf(fh2), Object.getPrototypeOf(fh));
+
+ assert.deepStrictEqual(await fh2.readFile(), await fs.readFile(__filename));
+ await fh2.close();
+
+ assert.rejects(() => fh.readFile(), { code: 'EBADF' });
+})().then(common.mustCall());
+
+(async function() {
+ // Check that there is no crash if the message is never read.
+ const fh = await fs.open(__filename);
+
+ const { port1 } = new MessageChannel();
+
+ assert.notStrictEqual(fh.fd, -1);
+ port1.postMessage(fh, [ fh ]);
+ assert.strictEqual(fh.fd, -1);
+})().then(common.mustCall());
+
+(async function() {
+ // Check that in the case of a context mismatch the message is discarded.
+ const fh = await fs.open(__filename);
+
+ const { port1, port2 } = new MessageChannel();
+
+ const ctx = vm.createContext();
+ const port2moved = moveMessagePortToContext(port2, ctx);
+ port2moved.onmessage = common.mustCall((msgEvent) => {
+ assert.strictEqual(msgEvent.data, 'second message');
+ port1.close();
+ });
+ port2moved.start();
+
+ assert.notStrictEqual(fh.fd, -1);
+ port1.postMessage(fh, [ fh ]);
+ assert.strictEqual(fh.fd, -1);
+
+ port1.postMessage('second message');
+})().then(common.mustCall());