Fixes #22995. Fixes #23000.
There were a handful of bugs here causing the hang (each with a
corresponding minimized test):
- We were canceling recv futures when `receiveMessageOnPort` was called,
but this caused the "receive loop" in the message port to exit. This was
due to the fact that `CancelHandle`s are never reset (i.e., once you
`cancel` a `CancelHandle`, it remains cancelled). That meant that after
`receieveMessageOnPort` was called, the subsequent calls to
`op_message_port_recv_message` would throw `Interrupted` exceptions, and
we would exit the loop.
The cancellation, however, isn't actually necessary.
`op_message_port_recv_message` only borrows the underlying port for long
enough to poll the receiver, so the borrow there could never overlap
with `op_message_port_recv_message_sync`.
- Calling `MessagePort.unref()` caused the "receive loop" in the message
port to exit. This was because we were setting
`messageEventListenerCount` to 0 on unref. Not only does that break the
counter when multiple `MessagePort`s are present in the same thread, but
we also exited the "receive loop" whenever the listener count was 0. I
assume this was to prevent the recv promise from keeping the event loop
open.
Instead of this, I chose to just unref the recv promise as needed to
control the event loop.
- The last bug causing the hang (which was a doozy to debug) ended up
being an unfortunate interaction between how we implement our
messageport "receive loop" and a pattern found in `npm:piscina` (which
angular uses). The gist of it is that piscina uses an atomic wait loop
along with `receiveMessageOnPort` in its worker threads, and as the
worker is getting started, the following incredibly convoluted series of
events occurs:
1. Parent sends a MessagePort `p` to worker
2. Parent sends a message `m` to the port `p`
3. Parent notifies the worker with `Atomics.notify` that a new message
is available
4. Worker receives message, adds "message" listener to port `p`
5. Adding the listener triggers `MessagePort.start()` on `p`
6. Receive loop in MessagePort.start receives the message `m`, but then
hits an await point and yields (before dispatching the "message" event)
7. Worker continues execution, starts the atomic wait loop, and
immediately receives the existing notification from the parent that a
message is available
8. Worker attempts to receive the new message `m` with
`receiveMessageOnPort`, but this returns `undefined` because the receive
loop already took the message in 6
9. Atomic wait loop continues to next iteration, waiting for the next
message with `Atomic.wait`
10. `Atomic.wait` blocks the worker thread, which prevents the receive
loop from continuing and dispatching the "message" event for the
received message
11. The parent waits for the worker to respond to the first message, and
waits
12. The thread can't make any more progress, and the whole process hangs
The fix I've chosen here (which I don't particularly love, but it works)
is to just delay the `MessagePort.start` call until the end of the event
loop turn, so that the atomic wait loop receives the message first. This
prevents the hang.
---
Those were the main issues causing the hang. There ended up being a few
other small bugs as well, namely `exit` being emitted multiple times,
and not patching up the message port when it's received by
`receiveMessageOnPort`.
Although using `--allow-run` without an allow list gives basically no
security, I think we should remove this warning because it gets in the
way and the only way to disable it is via --quiet.
Closes https://github.com/denoland/deno/issues/26012
```
========================================
failures:
"/WebCryptoAPI/wrapKey_unwrapKey/wrapKey_unwrapKey.https.any.html - Can wrap and unwrap X25519 private key keys as non-extractable using pkcs8 and AES-CTR"
final result: failed. 1 passed; 1 failed; 0 expected failure; total 2 (15646ms)
diff --git a/Users/divy/gh/deno/tests/wpt/runner/expectation.json b/var/folders/ll/ltqsx4nx72v5_r7rlsl36pgm0000gn/T/375d79f1257b05cd
index fb2063935..4449c5d15 100644
--- a/Users/divy/gh/deno/tests/wpt/runner/expectation.json
+++ b/var/folders/ll/ltqsx4nx72v5_r7rlsl36pgm0000gn/T/375d79f1257b05cd
@@ -1531,6 +1531,7 @@
},
"wrapKey_unwrapKey": {
"wrapKey_unwrapKey.https.any.html": [
+ "Can wrap and unwrap X25519 private key keys as non-extractable using pkcs8 and AES-CTR",
"Can wrap and unwrap X25519 private key keys as non-extractable using pkcs8 and AES-CBC",
"Can wrap and unwrap X25519 private key keys as non-extractable using pkcs8 and AES-GCM",
"Can wrap and unwrap X25519 private key keys as non-extractable using pkcs8 and AES-KW",
```
Fixes #25998. Fixes https://github.com/denoland/deno/issues/25928.
Originally I was just going to make this an error message instead of a
panic, but once I got to a minimal repro I felt that this really should
work.
The panic occurs when you have `nodeModulesDir: manual` (or a
package.json present), and you have an npm package with a tag in your
deno.json (see the spec test that illustrates this).
This code path only actually executes when trying to choose an
appropriate package version from `node_modules/.deno`, so we should be
able to fix it by storing some extra data at install time.
The fix proposed here is to repurpose the `.initialized` file that we
store in `node_modules` to store the tags associated with a package.
Basically, if you have a version requirement with a tag (e.g.
`npm:chalk@latest`), when we set up the node_modules folder for that
package, we store the tag (`latest`) in `.initialized`. Then, when doing
BYONM resolution, if we have a version requirement with a tag, we read
that file and check if the tag is present.
The downside is that we do more work when setting up `node_modules`. We
_could_ do this only when BYONM is enabled, but that would have the
downside of needing to re-run `deno install` when you switch from auto
-> manual, though maybe that's not a big deal.
Fixes #25861.
Previously we were attempting to match the version requirement against
the version already present in `node_modules` root, and if they didn't
match we would create a node_modules dir in the workspace member's
directory with the dependency.
Aside from the fact that this caused the panic, on second thought it
just doesn't make sense in general. We shouldn't be semver matching, as
resolution has already occurred and decided what package versions are
required. Instead, we can just compare the versions directly.
Fixes #24740.
Implements the `uv_mutex_*` and `uv_async_*` APIs.
The mutex API is implemented exactly as libuv, a thin wrapper over the
OS's native mutex.
The async API is implemented in terms of napi_async_work. As documented
in the napi docs, you really shouldn't call `napi_queue_async_work`
multiple times (it is documented as undefined behavior). However, our
implementation doesn't have any issue with this, so I believe it suits
our purpose here.
Previously the CLI was incorrectly reporting `React` as unused in a JSX
file that uses the "old" transform.
The LSP was already handling this correctly.
Currently we only warn once. With this PR, we continue to warn about
not-run scripts on explicit `deno install` (or cache). For `run` (or
other subcommands) we only warn the once, as we do currently.
Fixes https://github.com/denoland/deno/issues/25862.
npm only makes bin entries executable if they get linked into `.bin`, as
we did before this PR. So this PR actually deviates from npm, because
it's the only reasonable way to fix this that I can think of.
---
The reason this was broken in moment is the following:
Moment has dependencies on two typescript versions: 1.8 and 3.1
If you have two packages with conflicting bin entries (i.e. two
typescript versions which both have a bin entry `tsc`), in npm it is
non-deterministic and undefined which one will end up in `.bin`.
npm, due to implementation differences, chooses to put typescript 1.8
into the `.bin` directory, and so `node_modules/typescript/bin/tsc` ends
up getting marked executable. We, however, choose typescript 3.2, and so
we end up making `node_modules/typescript3/bin/tsc` executable.
As part of its tests, moment executes `node_modules/typescript/bin/tsc`.
Because we didn't make it executable, this fails.
Since the conflict resolution is undefined in npm, instead of trying to
match it, I think it makes more sense to just make bin entries
executable even if they aren't chosen in the case of a conflict.
This replaces `--allow-net` for import permissions and makes the
security sandbox stricter by also checking permissions for statically
analyzable imports.
By default, this has a value of
`--allow-import=deno.land:443,jsr.io:443,esm.sh:443,raw.githubusercontent.com:443,gist.githubusercontent.com:443`,
but that can be overridden by providing a different set of hosts.
Additionally, when no value is provided, import permissions are inferred
from the CLI arguments so the following works because
`fresh.deno.dev:443` will be added to the list of allowed imports:
```ts
deno run -A -r https://fresh.deno.dev
```
---------
Co-authored-by: David Sherret <dsherret@gmail.com>
Fixes #25813.
I initially tried doing this in `deno_semver`, where it's a cleaner
change, but that caused breakage in deno in places where we don't expect
a tag (see https://github.com/denoland/deno/issues/25857).
This does not fix wildcard requirements failing to choose pre-release
versions. That's a little more involved and I'll do a separate PR.
