worker: handle detached MessagePort from a different context

[juanarbol]

When worker.moveMessagePortToContext is used, the async handle associated with the port, will be triggered more than needed (at least one more time) and with null data. That can be avoided by simply checking that the data is present and the port is not detached.

Fixes: #49075

---

Note aside, this could be fixed as well by the next diff:

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
index 00e1db3bab..b46ae022ce 100644
--- a/src/node_messaging.cc+++ b/src/node_messaging.cc@@ -741,9 +741,6 @@ MessagePort* MessagePort::New(
// but it's better to be safe than sorry.)
Mutex::ScopedLock lock(port->data_->mutex_);
port->data_->owner_ = port;
- // If the existing MessagePortData object had pending messages, this is- // the easiest way to run that queue.- port->TriggerAsync();
} else if (sibling_group) {
sibling_group->Entangle(port->data_.get());
}

IMO that could change how the workers behave, then I prefer a "handle case" approach.

[nodejs-github-bot]

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

[juanarbol]

Cc @nodejs/workers 😅

[juanarbol]

@addaleax PTAL

[nodejs-github-bot]

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

[juanarbol]

Thanks for reviewing this!

[juanarbol]

@addaleax PTAL

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/49150
✔ Done loading data for nodejs/node/pull/49150
----------------------------------- PR info ------------------------------------
Title worker: handle detached `MessagePort` from a different context (#49150)
Author Juan José (@juanarbol)
Branch juanarbol:juan/fix-worker-issue -> nodejs:main
Labels c++, worker, needs-ci
Commits 1
- worker: handle detached `MessagePort` from a different context
Committers 1
- Juan José Arboleda PR-URL: https://github.com/nodejs/node/pull/49150
Fixes: https://github.com/nodejs/node/issues/49075
Reviewed-By: Anna Henningsen ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49150
Fixes: https://github.com/nodejs/node/issues/49075
Reviewed-By: Anna Henningsen --------------------------------------------------------------------------------
⚠ Commits were pushed since the last approving review:
⚠ - worker: handle detached `MessagePort` from a different context
ℹ This PR was created on Sun, 13 Aug 2023 19:44:53 GMT
✔ Approvals: 1
✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/49150#pullrequestreview-1593181662
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2023-09-05T16:34:36Z: https://ci.nodejs.org/job/node-test-pull-request/53747/
- Querying data for job/node-test-pull-request/53747/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/6092861198

[juanarbol]

Can anyone from @nodejs/workers review this one? Please, haha

[nodejs-github-bot]

Landed in c206b2a