GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Allow passing FileHandle objects via postMessage by addaleax · Pull Request #33772 · nodejs/node

Conversation

@addaleax

@addaleax addaleax commented Jun 6, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
src: introduce BaseObject base FunctionTemplate

(shared with #33360)

worker: allow transferring/cloning generic BaseObjects

Extend support for transferring objects à la MessagePort to other
types of BaseObject subclasses, as well as implement cloning
support for cases in which destructive transferring is not needed
or optional.

(shared with #33360)

fixup! worker: allow transferring/cloning generic BaseObjects

(not shared, hence as a fixup commit)

src: add equality operators for BaseObjectPtr
worker: allow passing JS wrapper objects via postMessage

Enable JS wrapper objects to be used as transferable or cloneable
objects in postMessage() calls, by having them extend a C++-backed
class.

This requires a few internal changes:

  • This commit adds the possibility for transferred objects to
    read/write JS values at the end of the serialization/deserialization
    phases.
  • This commit adds the possibility for transferred objects to list
    sub-transferables, e.g. typically the public JS wrapper class
    would list its C++ handle in there.
  • This commit adds usage of BaseObject in a few more places, because
    now during deserialization weakly held objects can also be involved,
    in addition to MessagePorts.
worker,fs: make FileHandle transferable

Allow passing FileHandle instances in the transfer list
of a .postMessage() call.

worker: emit 'messagerror' events for failed deserialization

This is much nicer than just treating exceptions as uncaught, and
enables reporting of exceptions from the internal C++ deserialization
machinery.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

himself65 and bricss reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@addaleax
addaleax requested review from jasnell and tniessen June 6, 2020 15:08
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs involving general changes in the lib/ or src/ directories. label Jun 6, 2020
addaleax
addaleax commented Jun 6, 2020
View reviewed changes
Comment thread src/node_crypto.cc Outdated

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tniessen Just a heads up, this is where the conflicts I mentioned to you came from

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/node_messaging.h Outdated
Comment on lines 29 to 32

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tniessen You will probably also want to include f172c2dc874a9f9575bd958e0d3eb828ced224ce in your PR. I missed the fact that returning a plain BaseObject* might not work in weird edge case situations, because objects like KeyObject or FileHandle are weak objects, and only holding a BaseObject* does not prevent them from being garbage-collected between returning them from Deserialize() and adding the corresponding JS object to the message data object.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/internal/fs/promises.js Outdated

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell Happy to take suggestions on the API here. It’s a bit unfortunate that we can’t use the constructor and instead use this deserialization method to fill the object’s contents with the transferred data, but I don’t really know a way around this (See the comment in JSTransferable::Data::Deserialize() for details on why it seems necessary to me)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@benjamingr benjamingr Jun 6, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is actually pretty common in JS for "hydrating" objects in my experience - though it is admittedly less neat than doing everything in the constructor and always getting a fully initialized object.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@jasnell jasnell Jun 8, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this API. The one thing that I would be concerned about is whether we'll always be certain that kDeserialize is always going to be called with a non-null/non-undefined value, otherwise the ({ handle }) deconstruction will fail.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@addaleax addaleax Jun 8, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will always be called with the value in [kTransfer]().data, so yes, that should work out.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread src/node_messaging.cc Outdated
Comment on lines 1217 to 1240

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tniessen I couldn’t figure out a good way to deal with this, sorry. I would suggest doing something similar in #33360, too, unless anybody has a good idea that doesn’t come with tons of extra complexity… on the plus side, this is not something that people are typically going to run into, and if they do, they are probably in a situation where they understand why the message can’t properly be transferred

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@tniessen tniessen Jun 7, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for explaining these changes, it's very helpful :-) I'll try to do the same in my PR :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@addaleax addaleax Jun 8, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tniessen See the comment below – the last commit here changes this to actually emit an error, so you could also do that in your PR :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@addaleax addaleax added fs Issues and PRs related to file-system APIs and the fs module. worker Issues and PRs related to the worker_threads module and Worker API. semver-minor PRs that contain new features and should be released in the next minor version. and removed lib / src Issues and PRs involving general changes in the lib/ or src/ directories. labels Jun 6, 2020
@nodejs-github-bot

nodejs-github-bot commented Jun 6, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31753/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr
benjamingr reviewed Jun 6, 2020
View reviewed changes
Comment thread lib/internal/fs/promises.js Outdated

@benjamingr benjamingr Jun 6, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have an impact on the memory consumption or size of FileHandle objects?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

himself65 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the inspector, this adds 32 bytes in total, which seems to make sense to me (1 × vpointer + 3 × pointer-size fields in BaseObject).

For comparison, the close function on each FileHandle is 56 bytes in size.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@benjamingr benjamingr Jun 6, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For comparison, the close function on each FileHandle is 56 bytes in size.

That is actually pretty unfortunate to allocate a function per handle, but I guess maybe file handles are expected to be relatively expensive to hold?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjamingr I would love to see our objects become smaller in general, but I think it’s fine here. There’s also the native FileHandle class, which also comes with 56 bytes per instance.

And as far as FileHandles as a resource are concerned, we maybe create thousands of them in a single process, but not millions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tniessen reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
benjamingr
benjamingr reviewed Jun 6, 2020
View reviewed changes
Comment thread lib/internal/worker/js_transferable.js Outdated

@benjamingr benjamingr Jun 6, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the module name be guarded with a whitelist here?

I don't think it should since worker threads don't provide a sandbox but I just want to make sure that this isn't exposing a new attack surface somehow.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@addaleax addaleax Jun 6, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjamingr Hm yeah … this isn’t extending capabilities of the calling code in any way, but I agree that it might be good to be careful here. I’ll think about this a bit more.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@addaleax addaleax Jun 7, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjamingr I’ve updated this PR so that this function now verifies that the target constructor is a subclass of JSTransferable, and added tests that verify that fact + the fact that this will only load internal modules (which was already the case before).

I think that’s as “locked down” as this can get. If user code makes other internal classes inherit from JSTransferable on the receiving side, and override the kTransfer method on the sending side – which is totally doable in most situations – then that means that you can already make arbitrary changes to Node.js internals and won’t even need to do this.

The one thing I could see happen would be side effects from require()ing the internal module itself, which the sending code can still trigger on the receiving side (and which would still typically result in an uncaught exception).

What do you think?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@benjamingr benjamingr Jun 7, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@benjamingr benjamingr Jun 8, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one thing I could see happen would be side effects from require()ing the internal module itself, which the sending code can still trigger on the receiving side (and which would still typically result in an uncaught exception).

Fwiw that's the part that initially bugged me, but like we both said it doesn't increase the attack surface at all from what I can tell.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
jasnell
jasnell reviewed Jun 8, 2020
View reviewed changes
Comment thread doc/api/worker_threads.md Outdated

@jasnell jasnell Jun 8, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, we might want to consider marking transferable objects with a special static read-only symbol property so we don't actually have to maintain this list. For instance, something like:

const { transferableSymbol } = require('worker_threads');

MessagePort[transferableSymbol]         // True!
FileHandle[transferableSymbol]          // True!
MyOtherHandleThing[transferableSymbol   // False!

Then we wouldn't necessarily have to maintain a list of such objects in the docs, we would just say that value may be any object where [transferableSymbol] === true

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr and addaleax reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@addaleax addaleax Jun 8, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell I think that’s a good idea, but I do think it’s important to list the built-in types that are transferable here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos and benjamingr reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@jasnell jasnell Jun 8, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I think we could put those in one place rather than having them listed in a couple different places (they're currently listed in the value and the transferList, etc)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@benjamingr

benjamingr commented Jun 8, 2020

Copy link
Copy Markdown
Member

So the actual changes LGTM (ran fine when I checked them, the feature itself is pretty cool). I'm not sure that the cost of an additional 32 bytes per handle is worth it - but that's something we can probably fix later.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@addaleax

addaleax commented Jun 8, 2020

Copy link
Copy Markdown
Member Author

@benjamingr @tniessen Fwiw, I’ve pushed one additional commit here – it really bugged me that exceptions from deserialization would always end up as uncaught exceptions, and that we had no way of informing users about errors in the case of e.g. a context mismatch. I’ve made the MessagePort emit messagerror events in that case (That’s the event name used by the Web MessagePorts as well). In particular, this means that the sending side can’t really trigger uncaught exceptions on the receiving side in most cases.

Could you take a look at that as well?

benjamingr reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Jun 8, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31776/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@addaleax addaleax added the review wanted PRs that need review. label Jun 8, 2020
benjamingr
benjamingr reviewed Jun 8, 2020
View reviewed changes
Comment thread src/node_messaging.cc Outdated

@benjamingr benjamingr Jun 8, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to emit error or uncaughtException if no one is listening to messageerror?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@benjamingr benjamingr Jun 8, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or a warning)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@addaleax addaleax Jun 8, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjamingr I would prefer to drop the message in that case, rather than start emitting more events/raise it as an uncaught exception… I think if anything, a warning would be okay, but like you said below, this isn’t something that should really occur in normal flow (and I assume you meant to refer to that as not critical)?

So yeah, if you feel strongly about it, I’d add a warning, otherwise I’d probably say it’s not worth the extra complexity.

(Also, I don’t know if your LGTM counts as an approval of this PR?)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@benjamingr

benjamingr commented Jun 8, 2020

Copy link
Copy Markdown
Member

Could you take a look at that as well?

I think it makes sense although I think people shouldn't run into deserialization errors in a normal flow and it's probably critical.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr
benjamingr approved these changes Jun 8, 2020
View reviewed changes

@benjamingr benjamingr left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making my LGTM explicit

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot

nodejs-github-bot commented Jun 8, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31785/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jun 9, 2020
CI Relability 2020-06-09 JaneaSystems/nodejs-reliability#9
Open
CI Relability 2020-06-10 JaneaSystems/nodejs-reliability#10
Open
@nodejs-github-bot

nodejs-github-bot commented Jun 11, 2020 •
edited by addaleax
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31837/ (:yellow_heart:)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax added 6 commits June 11, 2020 23:54
@addaleax
src: introduce BaseObject base FunctionTemplate
cbc35c3
@addaleax
worker: allow transferring/cloning generic BaseObjects
915b90e
Extend support for transferring objects à la `MessagePort` to other
types of `BaseObject` subclasses, as well as implement cloning
support for cases in which destructive transferring is not needed
or optional.
@addaleax
fixup! worker: allow transferring/cloning generic BaseObjects
5f25ff9
@addaleax
src: add equality operators for BaseObjectPtr
b1aad92
@addaleax
worker: allow passing JS wrapper objects via postMessage
877d631
Enable JS wrapper objects to be used as transferable or cloneable
objects in `postMessage()` calls, by having them extend a C++-backed
class.

This requires a few internal changes:
- This commit adds the possibility for transferred objects to
  read/write JS values at the end of the serialization/deserialization
  phases.
- This commit adds the possibility for transferred objects to list
  sub-transferables, e.g. typically the public JS wrapper class
  would list its C++ handle in there.
- This commit adds usage of `BaseObject` in a few more places, because
  now during deserialization weakly held objects can also be involved,
  in addition to `MessagePort`s.
@addaleax
worker,fs: make FileHandle transferable
758fe1a
Allow passing `FileHandle` instances in the transfer list
of a `.postMessage()` call.
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@addaleax @codebytere
src: introduce BaseObject base FunctionTemplate
f50febc
PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@addaleax @codebytere
worker: allow transferring/cloning generic BaseObjects
11daf8b
Extend support for transferring objects à la `MessagePort` to other
types of `BaseObject` subclasses, as well as implement cloning
support for cases in which destructive transferring is not needed
or optional.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@addaleax @codebytere
src: add equality operators for BaseObjectPtr
8be7c37
PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@addaleax @codebytere
worker: allow passing JS wrapper objects via postMessage
c28726b
Enable JS wrapper objects to be used as transferable or cloneable
objects in `postMessage()` calls, by having them extend a C++-backed
class.

This requires a few internal changes:
- This commit adds the possibility for transferred objects to
  read/write JS values at the end of the serialization/deserialization
  phases.
- This commit adds the possibility for transferred objects to list
  sub-transferables, e.g. typically the public JS wrapper class
  would list its C++ handle in there.
- This commit adds usage of `BaseObject` in a few more places, because
  now during deserialization weakly held objects can also be involved,
  in addition to `MessagePort`s.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@addaleax @codebytere
worker,fs: make FileHandle transferable
32bb67a
Allow passing `FileHandle` instances in the transfer list
of a `.postMessage()` call.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@addaleax @codebytere
worker: emit 'messagerror' events for failed deserialization
6cd924d
This is much nicer than just treating exceptions as uncaught, and
enables reporting of exceptions from the internal C++ deserialization
machinery.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere added a commit that referenced this pull request Jun 30, 2020
@codebytere
2020-06-30, Version 14.5.0 (Current)
abb5e1a
Notable changes:

build:
  * (SEMVER-MINOR) reset embedder string to "-node.0" (Michaël Zasso) #33376
cli:
  * (SEMVER-MINOR) add alias for report-directory to make it consistent (AshCripps) #33587
crypto:
  * (SEMVER-MINOR) allow KeyObjects in postMessage (Tobias Nießen) #33360
deps:
  * (SEMVER-MINOR) V8: cherry-pick 0d6debcc5f08 (Michaël Zasso) #33376
  * (SEMVER-MINOR) update V8 to 8.3.110.9 (Michaël Zasso) #33376
dgram:
  * (SEMVER-MINOR) allow typed arrays in .send() (Sarat Addepalli) #22413
events:
  * (SEMVER-MINOR) initial implementation of experimental EventTarget (James M Snell) #33556
fs:
  * (SEMVER-MINOR) implement lutimes (Maël Nison) #33399
http:
  * (SEMVER-MINOR) expose host and protocol on ClientRequest (wenningplus) #33803
  * (SEMVER-MINOR) add maxTotalSockets to agent class (rickyes) #33617
  * (SEMVER-MINOR) return this from OutgoingMessage#destroy() (Colin Ihrig) #32789
  * (SEMVER-MINOR) return this from ClientRequest#destroy() (Colin Ihrig) #32789
  * (SEMVER-MINOR) return this from IncomingMessage#destroy() (Colin Ihrig) #32789
  * (SEMVER-MINOR) added scheduling option to http agent (delvedor) #33278
http2:
  * (SEMVER-MINOR) return this for Http2ServerRequest#setTimeout (Pranshu Srivastava) #33994
  * (SEMVER-MINOR) do not modify explicity set date headers (Pranshu Srivastava) #33160
process:
  * (SEMVER-MINOR) add unhandled-rejection throw and warn-with-error-code (Dan Fabulich) #33475
src:
  * (SEMVER-MINOR) store key data in separate class (Tobias Nießen) #33360
  * (SEMVER-MINOR) add NativeKeyObject base class (Tobias Nießen) #33360
  * (SEMVER-MINOR) rename internal key handles to KeyObjectHandle (Tobias Nießen) #33360
  * (SEMVER-MINOR) add equality operators for BaseObjectPtr (Anna Henningsen) #33772
  * (SEMVER-MINOR) introduce BaseObject base FunctionTemplate (Anna Henningsen) #33772
  * (SEMVER-MINOR) add public APIs to manage v8::TracingController (Anna Henningsen) #33850
stream*:
  * runtime deprecate Transform._transformState (Robert Nagy) #32763
win:
  * (SEMVER-MINOR) allow skipping the supported platform check (João Reis) #33176
worker:
  * (SEMVER-MINOR) add public method for marking objects as untransferable (Anna Henningsen) #33979
  * (SEMVER-MINOR) emit `'messagerror'` events for failed deserialization (Anna Henningsen) #33772
  * (SEMVER-MINOR) allow passing JS wrapper objects via postMessage (Anna Henningsen) #33772
  * (SEMVER-MINOR) allow transferring/cloning generic BaseObjects (Anna Henningsen) #33772
worker,fs:
  * (SEMVER-MINOR) make FileHandle transferable (Anna Henningsen) #33772
zlib:
  * (SEMVER-MINOR) add `maxOutputLength` option (unknown) #33516

PR-URL: #34093
codebytere added a commit that referenced this pull request Jun 30, 2020
@codebytere
2020-06-30, Version 14.5.0 (Current)
eb8fc2b
Notable changes:

build:
  * (SEMVER-MINOR) reset embedder string to "-node.0" (Michaël Zasso) #33376
cli:
  * (SEMVER-MINOR) add alias for report-directory to make it consistent (AshCripps) #33587
crypto:
  * (SEMVER-MINOR) allow KeyObjects in postMessage (Tobias Nießen) #33360
deps:
  * (SEMVER-MINOR) V8: cherry-pick 0d6debcc5f08 (Michaël Zasso) #33376
  * (SEMVER-MINOR) update V8 to 8.3.110.9 (Michaël Zasso) #33376
dgram:
  * (SEMVER-MINOR) allow typed arrays in .send() (Sarat Addepalli) #22413
events:
  * (SEMVER-MINOR) initial implementation of experimental EventTarget (James M Snell) #33556
fs:
  * (SEMVER-MINOR) implement lutimes (Maël Nison) #33399
http:
  * (SEMVER-MINOR) expose host and protocol on ClientRequest (wenningplus) #33803
  * (SEMVER-MINOR) add maxTotalSockets to agent class (rickyes) #33617
  * (SEMVER-MINOR) return this from OutgoingMessage#destroy() (Colin Ihrig) #32789
  * (SEMVER-MINOR) return this from ClientRequest#destroy() (Colin Ihrig) #32789
  * (SEMVER-MINOR) return this from IncomingMessage#destroy() (Colin Ihrig) #32789
  * (SEMVER-MINOR) added scheduling option to http agent (delvedor) #33278
http2:
  * (SEMVER-MINOR) return this for Http2ServerRequest#setTimeout (Pranshu Srivastava) #33994
  * (SEMVER-MINOR) do not modify explicity set date headers (Pranshu Srivastava) #33160
process:
  * (SEMVER-MINOR) add unhandled-rejection throw and warn-with-error-code (Dan Fabulich) #33475
src:
  * (SEMVER-MINOR) store key data in separate class (Tobias Nießen) #33360
  * (SEMVER-MINOR) add NativeKeyObject base class (Tobias Nießen) #33360
  * (SEMVER-MINOR) rename internal key handles to KeyObjectHandle (Tobias Nießen) #33360
  * (SEMVER-MINOR) add equality operators for BaseObjectPtr (Anna Henningsen) #33772
  * (SEMVER-MINOR) introduce BaseObject base FunctionTemplate (Anna Henningsen) #33772
  * (SEMVER-MINOR) add public APIs to manage v8::TracingController (Anna Henningsen) #33850
stream*:
  * runtime deprecate Transform._transformState (Robert Nagy) #32763
win:
  * (SEMVER-MINOR) allow skipping the supported platform check (João Reis) #33176
worker:
  * (SEMVER-MINOR) add public method for marking objects as untransferable (Anna Henningsen) #33979
  * (SEMVER-MINOR) emit `'messagerror'` events for failed deserialization (Anna Henningsen) #33772
  * (SEMVER-MINOR) allow passing JS wrapper objects via postMessage (Anna Henningsen) #33772
  * (SEMVER-MINOR) allow transferring/cloning generic BaseObjects (Anna Henningsen) #33772
worker,fs:
  * (SEMVER-MINOR) make FileHandle transferable (Anna Henningsen) #33772
zlib:
  * (SEMVER-MINOR) add `maxOutputLength` option (unknown) #33516

PR-URL: #34093
@addaleax addaleax added backported-to-v14.x and removed backport-open-v14.x labels Aug 17, 2020
addaleax added a commit that referenced this pull request Sep 22, 2020
@addaleax
src: add equality operators for BaseObjectPtr
19b55be
PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 27, 2020
@addaleax
src: introduce BaseObject base FunctionTemplate
667d520
PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 27, 2020
@addaleax
worker: allow transferring/cloning generic BaseObjects
8e1698a
Extend support for transferring objects à la `MessagePort` to other
types of `BaseObject` subclasses, as well as implement cloning
support for cases in which destructive transferring is not needed
or optional.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 27, 2020
@addaleax
worker: allow passing JS wrapper objects via postMessage
0d35eaa
Enable JS wrapper objects to be used as transferable or cloneable
objects in `postMessage()` calls, by having them extend a C++-backed
class.

This requires a few internal changes:
- This commit adds the possibility for transferred objects to
  read/write JS values at the end of the serialization/deserialization
  phases.
- This commit adds the possibility for transferred objects to list
  sub-transferables, e.g. typically the public JS wrapper class
  would list its C++ handle in there.
- This commit adds usage of `BaseObject` in a few more places, because
  now during deserialization weakly held objects can also be involved,
  in addition to `MessagePort`s.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 27, 2020
@addaleax
worker,fs: make FileHandle transferable
64f1ec5
Allow passing `FileHandle` instances in the transfer list
of a `.postMessage()` call.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 27, 2020
@addaleax
worker: emit 'messagerror' events for failed deserialization
eec836e
This is much nicer than just treating exceptions as uncaught, and
enables reporting of exceptions from the internal C++ deserialization
machinery.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 28, 2020
@addaleax
worker,fs: make FileHandle transferable
dd51ba3
Allow passing `FileHandle` instances in the transfer list
of a `.postMessage()` call.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax added a commit that referenced this pull request Sep 28, 2020
@addaleax
worker: emit 'messagerror' events for failed deserialization
dafa380
This is much nicer than just treating exceptions as uncaught, and
enables reporting of exceptions from the internal C++ deserialization
machinery.

PR-URL: #33772
Backport-PR-URL: #33965
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere added a commit that referenced this pull request Sep 28, 2020
@codebytere
2020-10-06, Version 12.19.0 'Erbium' (LTS)
38e875d
Notable changes:

async_hooks:
  * add AsyncResource.bind utility (James M Snell) (#34574)
buffer:
  * also alias BigUInt methods (Anna Henningsen) (#34960)
  * alias UInt ➡️ Uint in buffer methods (Anna Henningsen) (#34729)
build:
  * add build flag for OSS-Fuzz integration (davkor) (#34761)
cli:
  * add alias for report-directory to make it consistent (Ash Cripps) (#33587)
crypto:
  * allow KeyObjects in postMessage (Tobias Nießen) (#33360)
  * add randomInt function (Oli Lalonde) (#34600)
deps:
  * upgrade to libuv 1.39.0 (Colin Ihrig) (#34915)
dgram:
  * add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) (#14500)
  * allow typed arrays in .send() (Sarat Addepalli) (#22413)
doc:
  * add basic embedding example documentation (Anna Henningsen) (#30467)
embedding:
  * make Stop() stop Workers (Anna Henningsen) (#32531)
  * provide hook for custom process.exit() behaviour (Anna Henningsen) (#32531)
fs:
  * implement lutimes (Maël Nison) (#33399)
http:
  * return this from IncomingMessage#destroy() (Colin Ihrig) (#32789)
  * expose host and protocol on ClientRequest (wenningplus) [#33803](#33803)
http2:
  * return this for Http2ServerRequest#setTimeout (Pranshu Srivastava) (#33994)
  * do not modify explicity set date headers (Pranshu Srivastava) (#33160)
* n-api**:
  * support type-tagging objects (Gabriel Schulhof) (#28237)
  * provide asynchronous cleanup hooks (Anna Henningsen) (#34572)
perf_hooks:
  * add idleTime and event loop util (Trevor Norris) (#34938)
timers:
  * allow timers to be used as primitives (Denys Otrishko) [#34017](#34017)
tls:
  * make 'createSecureContext' honor more options (Mateusz Krawczuk) (#33974)
worker:
  * add public method for marking objects as untransferable (Anna Henningsen) (#33979)
  * emit `'messagerror'` events for failed deserialization (Anna Henningsen) (#33772)
  * allow passing JS wrapper objects via postMessage (Anna Henningsen) (#33772)
  * allow transferring/cloning generic BaseObjects (Anna Henningsen) (#33772)
  * add option to track unmanaged file descriptors (Anna Henningsen) (#34303)
  * add stack size resource limit option (Anna Henningsen) (#33085)
  * make FileHandle transferable (Anna Henningsen) (#33772)
zlib:
  * add `maxOutputLength` option (unknown) (#33516)

PR-URL: TODO
@codebytere codebytere mentioned this pull request Sep 28, 2020
v12.19.0 proposal #35401
Merged
@Semigradsky Semigradsky mentioned this pull request Oct 2, 2020
[node] Update worker_threads module DefinitelyTyped/DefinitelyTyped#48424
Merged
@skyclouds2001 skyclouds2001 mentioned this pull request Nov 9, 2024
Node.js 11.7+ supports Worker + related updates mdn/browser-compat-data#25012
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell left review comments
@benjamingr benjamingr benjamingr approved these changes
@tniessen tniessen Awaiting requested review from tniessen

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

fs Issues and PRs related to file-system APIs and the fs module. review wanted PRs that need review. semver-minor PRs that contain new features and should be released in the next minor version. worker Issues and PRs related to the worker_threads module and Worker API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@addaleax @nodejs-github-bot @benjamingr @codebytere @jasnell @tniessen
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.