doc: fix wording in napi_async_init - #49180
Conversation
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
legendecas
left a comment
There was a problem hiding this comment.
The grammatical fix is LGTM.
Sorry, something went wrong.
| In order to retain ABI compatibility with previous versions, passing `NULL` | ||
| for `async_resource` does not result in an error. However, this is not | ||
| recommended as this will result poor results with `async_hooks` | ||
| recommended as this will result in undesirable behavior with `async_hooks` |
There was a problem hiding this comment.
I found that in #32930, NULL is coerced to a newly created strong-referenced object to avoid breaking async_hooks.executionAsyncResource(). So it is not breaking async_hooks.executionAsyncResource() and AsyncLocalStorage, with the reason mentioned in the paragraph above.
As async_hooks.executionAsyncResource() is guaranteed to return a non-null object, it might be sufficient to state it is undefined behavior if the async_resource is NULL with the semantic conflicts.
Sorry, something went wrong.
mhdawson
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
Refs: nodejs/node#32930 PR-URL: nodejs/node#49180 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Refs: nodejs/node#32930 PR-URL: nodejs/node#49180 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Even after reading through #32930, I am not quite sure what this was supposed to say, so please feel free to suggest alternatives. cc @legendecas @mhdawson