Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge with upstream #66

Merged
merged 17 commits into from
Dec 11, 2023
Merged

Merge with upstream #66

merged 17 commits into from
Dec 11, 2023

Conversation

dhil
Copy link
Member

@dhil dhil commented Dec 11, 2023

No description provided.

saulecabrera and others added 17 commits December 5, 2023 10:36
This commit reworks the `br_table` logic so that it correctly handles
all the jumps involved to each of the targets.

Even though it is safe to use the default branch for type information,
it is not safe to use it to derive the base stack pointer and base value
stack length. This change ensures that each target offset is taken into
account to balance the value stack prior to each jump.
* Rewrite wait/notify with wasm threads

This commit rewrites and refactors the `ParkingSpot` implementation in
Wasmtime. This is motivated by bytecodealliance#7623 primarily which is something I haven't
been able to reproduce but it doesn't look like a spurious issue. Additionally
in reviewing the previous implementation I'm not sure if it was technically
spec-compliant.

Previously each parking spot was modeled with a single condition variable. All
threads blocking on the same address would block on the same condition
variable. When waking up N threads the condition variable would either use
`notify_all` or `notify_one` N times. The part I wasn't so sure about is that
each thread, when waking up, would "consume" a wakeup notification on the way
out, going back to sleep if a notification wasn't available. This was intended
to handle spurious wakeups from the OS condition variable in use. This could
mean, however, that a spurious wakeup of one thread could consume a
notification from another thread. This was documented already as a possibility
and "probably ok" but my gut is that this behavior led to bytecodealliance#7623, although I
haven't been able to construct a trace that would lead the test here to
deadlock.

Out of caution, however, this commit rewrites the implementation to be similar
to what V8 and SpiderMonkey are doing. Both of those implementations are using
a linked list of waiters for threads that are blocked and then notifying
N threads dequeues N threads to wake them up. This makes the semantics
of knowing which thread is waken up easier to understand from an
implementation point of view since each wakeup notification
deterministically goes to a specified thread. The tricky part about this
implementation is that a doubly-linked-list needs to be maintained
within `ParkingSpot` to handle this.

While I was here I additionally refactored the interface of
`ParkingSpot` to more closely match the raw interface of WebAssembly.
This is intended to scope the problem being solved more narrowly to what
wasm needs rather than trying to solve a more general problem at the
same time.

Closes bytecodealliance#7623

* Touch up some comments
I realized just now that we haven't actually documented our usage of
`cargo vet` anywhere in our contributing documentation (or not that I
could find), so I decided to try and rectify that!
This change deduplicates some module documentation the pooling
allocator. Previously, the module-level documentation for `pooling.rs`
and `memory_pool.rs` had duplicated content which this change removes.
It also takes a stab at clarifying "how" and "why" MPK is used for
memory pooling.
* wasi-cli: update to version 0.2.0-rc-2023-12-05

* wasi-http: update to 0.2.0-rc-2023-12-05

* fix versions in extra (non-wasi) wits

* component adapter: fixes to use cli/imports world, correct versions

* wasmtime-wasi: cli/reactor is now cli/imports

* sync wasi-http/wit with wasi/wit

* fix cli-test component-basic.wat version
This commit is an attempt to address the CI failure popping up in bytecodealliance#7636.
I can reproduce the failure locally and it appears to be due to the fact
that macOS is giving spawned threads via `pthread_create` a 512K stack
by default. Cranelift when compiled in debug mode is taking more stack
than this (but working in release mode). I was talking with Trevor and
Jamey about possible ways to reduce the stack size here but for now I'm
going with Jamey's suggestion of increasing the stack size as the best
course of action for now.
…alliance#7636)

* Add opt patterns for 3-way comparison (`Ord::cmp` or `<=>`)

Compare clang: <https://cpp.godbolt.org/z/bTbe1556W>

* Add `spaceship_[su]` extractors and constructors
…lliance#7647)

* Add options to WasiCtx for toggling TCP and UDP on and off

Signed-off-by: Ryan Levick <[email protected]>

* Address PR feedback

Signed-off-by: Ryan Levick <[email protected]>

* Add test for turning off tcp

Signed-off-by: Ryan Levick <[email protected]>

---------

Signed-off-by: Ryan Levick <[email protected]>
…ecodealliance#7655)

* rename to ResourceTable, docs, and export

* wasi: use the table from wasmtime::component

* wasi-http: use wasmtime::component's resource table now

* rename file (somehow got lost)

* wasmtime-cli: fixes for resourcetable

* fix wasi-http tests

* more test fixes

* error messages

* fix warning

* cli: fix min build

prtest:full
…alliance#7648)

* Ensure remote_address is allowed when creating UDP stream

Signed-off-by: Ryan Levick <[email protected]>

* Check addr validity after UDP state check and wrap pool in Arc

Signed-off-by: Ryan Levick <[email protected]>

* Check provided addr in outgoing stream

Signed-off-by: Ryan Levick <[email protected]>

* Add test for turning off udp

Signed-off-by: Ryan Levick <[email protected]>

* Model pool on UDP types as optional to catch bad state bugs

Signed-off-by: Ryan Levick <[email protected]>

---------

Signed-off-by: Ryan Levick <[email protected]>
The test simply tries looking up example.com and ensuring it fails with
`PermanentResolverFailure`. Additionally, `PermanentResolverFailure` is
removed as one of the allowed errors in the normal ip name lookup test
as those allowed errors should only be transient lookup errors and
`PermanentResolverFailure` is very much not a transient error.

Signed-off-by: Ryan Levick <[email protected]>
…dealliance#7663)

* add `adapter_{open|close}_badfd` exports to Preview 1 adapter

This is to be used by `wasi-libc` to reserve file descriptors for its own use
(e.g. for sockets), ensuring that any attempt to pass them directly to Preview 1
functions will consistently return an error.

See WebAssembly/wasi-libc#447 for further details.

Signed-off-by: Joel Dice <[email protected]>

* add `preview2_adapter_badfd` test

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>
@dhil dhil merged commit 6ed3e1d into wasmfx:main Dec 11, 2023
4 checks passed
@dhil dhil deleted the wasmfx-merge branch December 11, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants