Implement async termination of blocking thread #2516

yamt · 2023-08-30T08:31:16Z

send a signal whose handler is no-op to a blocking thread to wake up the blocking syscall
with either EINTR equivalent or partial success.

unlike the approach taken in the dev/interrupt_block_insn branch (that is, signal + longjmp similarly to OS_ENABLE_HW_BOUND_CHECK),
this PR does not use longjmp because:

longjmp from signal handler doesn't work on nuttx (signal handler is called within "kernel" apache/nuttx#10326)
i'm afraid that the singal+longjmp approach is too difficult for average programmers who might implement host functions to deal with. especially when the longjmp can rewind a lot of functions, some of which are potentially user-supplied.

todo/known issues/limitations/open questions:

~~make the initialization independent from OS_ENABLE_HW_BOUND_CHECK~~
~~provide an api (or at least a build-time config) to specifiy a signal to use, instead of hardcoding SIGUSR1.~~
adapt operations other than fd_read
- ~~fd_close~~
- ~~fd_pread~~
- ~~fd_pwrite~~
- ~~fd_read~~
- ~~fd_write~~
- ~~path_open~~
  - i guess it can block on eg. fifo. i'm not sure if i want to fix it in this PR.
- poll_oneoff
  - leave this for now as it's already fixed with a different approach: modified poll_oneoff to make it interruptible #1951
- ~~sock_accept~~
- ~~sock_close (?)~~ just ENOSYS
- ~~sock_connect~~
- ~~sock_recv~~ just a wrapper of sock_recv_from
- ~~sock_recv_from~~
- ~~sock_send~~ just a wrapper of sock_send_to
- ~~sock_send_to~~
- ~~sock_shutdown~~ usually not block
- ~~sock_addr_resolve~~
  - this one is not a simple syscall. see the comment in blocking_op.c
- memory.atomic.wait32/64
  - leave this for now as it's already fixed with a different approach: Fix atomic.wait, get wasi_ctx exit code and thread mgr issues #2024
what to do for platforms other than posix?
what to do for other wasi implementation? (uvwasi)
~~document/comment~~
test
- the relevant tests in wasi-threads repo passed on macOS and nuttx/esp32-devkitc
~~add an api to terminate an instance~~ Add an API to terminate instance #2538
~~add a sample~~ Add an API to terminate instance #2538
for simple blocking system call like readv, it's easy to wrap it with wasm_runtime_begin_blocking_op/wasm_runtime_end_blocking_op. but how about something more complex, like a call which possibly blocks deep inside an external library?

relevant issue: #1910

yamt · 2023-08-31T23:02:42Z

the last push passes all test cases in wasi-threads repo on macOS.

yamt · 2023-09-13T12:43:55Z

the ci failure looks unrelated

  Cannot initiate the connection to ppa.launchpadcontent.net:443 (2620:2d:4000:1::3e). - connect (101: Network is unreachable) Could not connect to ppa.launchpadcontent.net:443 (185.125.190.52), connection timed out [IP: 185.125.190.52 443]

core/iwasm/include/wasm_export.h

core/shared/platform/common/posix/posix_blocking_op.c

wenyongh · 2023-09-18T04:01:43Z

@eloparco Could you help review the PR? It implements feature like dev/interrupt_block_insn in different manner.

basically a port from https://github.com/yamt/garbage/blob/master/c/sig2/test.c this ported version has not been tested yet though.

for now, only update fd_read. it should be easy to apply to other methods as the necessary change is mostly mechanical.

I was assuming pthread_sigmask was an alias of sigprocmask. It's true for platforms I'm familiar with. However, after reading the descriptions in the relevant standards, I changed my mind.

eloparco · 2023-09-19T09:55:14Z

@eloparco Could you help review the PR? It implements feature like dev/interrupt_block_insn in different manner.

Sure, I'll take a look in the next couple of days.

core/iwasm/common/wasm_blocking_op.c

wenyongh

LGTM

eloparco · 2023-09-19T23:59:02Z

Took a look. This doesn't cover user-defined functions that could have been achieved with longjmp I think. But, as you said, the longjmp approach is probably too complicated.

We should be able to add these tests now, probably in a separate PR.

yamt · 2023-09-20T00:25:50Z

Took a look. This doesn't cover user-defined functions that could have been achieved with longjmp I think. But, as you said, the longjmp approach is probably too complicated.

user-defined functions should take care of it by itself, either using the api introduced by this PR or other approaches like #1951. i guess in many cases it's simpler than preparing for signal+longjmp.

We should be able to add these tests now, probably in a separate PR.

yes. they all pass with this PR: #2516 (comment)

wenyongh · 2023-09-20T08:43:58Z

Took a look. This doesn't cover user-defined functions that could have been achieved with longjmp I think. But, as you said, the longjmp approach is probably too complicated.

We should be able to add these tests now, probably in a separate PR.

@eloparco so the PR is good to you, right? Shall we merge the PR?

eloparco · 2023-09-20T08:53:03Z

@eloparco so the PR is good to you, right? Shall we merge the PR?

LGTM

On posix-like platforms, the rest of wasi-threads tests should pass after the recent changes including the following PRs: bytecodealliance#2516 bytecodealliance#2524 bytecodealliance#2529

Implement async termination of blocking thread (bytecodealliance#2516)

On posix-like platforms, the rest of wasi-threads tests should pass after the recent changes including the following PRs: bytecodealliance#2516 bytecodealliance#2524 bytecodealliance#2529

On posix-like platforms, the rest of wasi-threads tests should pass after the recent changes including the following PRs: #2516, #2524, #2529, #2571, #2576 and #2582.

Send a signal whose handler is no-op to a blocking thread to wake up the blocking syscall with either EINTR equivalent or partial success. Unlike the approach taken in the `dev/interrupt_block_insn` branch (that is, signal + longjmp similarly to `OS_ENABLE_HW_BOUND_CHECK`), this PR does not use longjmp because: * longjmp from signal handler doesn't work on nuttx refer to apache/nuttx#10326 * the singal+longjmp approach may be too difficult for average programmers who might implement host functions to deal with See also bytecodealliance#1910

On posix-like platforms, the rest of wasi-threads tests should pass after the recent changes including the following PRs: bytecodealliance#2516, bytecodealliance#2524, bytecodealliance#2529, bytecodealliance#2571, bytecodealliance#2576 and bytecodealliance#2582.

yamt mentioned this pull request Aug 30, 2023

Implement os_usleep for posix #2517

Merged

yamt changed the title ~~Implement termination of blocking thread~~ Implement async termination of blocking thread Aug 30, 2023

yamt force-pushed the blocking-op branch 3 times, most recently from 38f769a to 455061a Compare August 31, 2023 22:59

yamt force-pushed the blocking-op branch 2 times, most recently from 70a66cb to 8b14c2f Compare September 4, 2023 05:30

yamt mentioned this pull request Sep 4, 2023

handle a return from wasi _start function correctly #2529

Merged

yamt force-pushed the blocking-op branch 4 times, most recently from 348a15d to 1b28087 Compare September 7, 2023 08:41

yamt mentioned this pull request Sep 8, 2023

Add an API to terminate instance #2538

Merged

yamt force-pushed the blocking-op branch 2 times, most recently from c3a7e6f to 4877ae8 Compare September 13, 2023 11:45

yamt marked this pull request as ready for review September 13, 2023 11:46

wenyongh reviewed Sep 18, 2023

View reviewed changes

core/iwasm/include/wasm_export.h Outdated Show resolved Hide resolved

core/iwasm/include/wasm_export.h Outdated Show resolved Hide resolved

core/iwasm/include/wasm_export.h Outdated Show resolved Hide resolved

core/shared/platform/common/posix/posix_blocking_op.c Show resolved Hide resolved

yamt added 10 commits September 19, 2023 11:37

some logic to terminate blocking op

53f1655

basically a port from https://github.com/yamt/garbage/blob/master/c/sig2/test.c this ported version has not been tested yet though.

avoid sleeping with a mutex held

5ea4c03

comment

6fbe85c

fix an inverted condition

63bdb21

wasm_export.h: add wasm_runtime_{begin|end}_blocking_op

1bfc737

make set_thread_cancel_flags interrupt blocking op

ec28fa2

wasi: mark blocking ops explicitly

acfd426

for now, only update fd_read. it should be easy to apply to other methods as the necessary change is mostly mechanical.

separate blocking op initialization from thread signal init

3114f90

nuttx: update the file list

0dbcee3

wasm_blocking_op.c: this stuff doesn't make sense w/o threads

f8c8070

yamt added 7 commits September 19, 2023 11:37

comment

a770e37

add os_set_signal_number_for_blocking_op

6643822

comment

33d23f0

posix_blocking_op.c: use pthread_sigmask instead of sigprocmask

143a7d6

I was assuming pthread_sigmask was an alias of sigprocmask. It's true for platforms I'm familiar with. However, after reading the descriptions in the relevant standards, I changed my mind.

add a comment about getaddrinfo

fd13caf

comment

5ac3cb5

comments

0daf3d7

yamt force-pushed the blocking-op branch from 8345b24 to 0daf3d7 Compare September 19, 2023 02:37

yamt added 3 commits September 19, 2023 11:38

comment

b5399cd

add a build-time config WAMR_DISABLE_WAKEUP_BLOCKING_OP

ecf798a

add messages

3be54a0

wenyongh reviewed Sep 19, 2023

View reviewed changes

core/iwasm/common/wasm_blocking_op.c Show resolved Hide resolved

add a missing unlock

778d2b2

wenyongh reviewed Sep 19, 2023

View reviewed changes

wenyongh merged commit 444b159 into bytecodealliance:main Sep 20, 2023

yamt mentioned this pull request Sep 20, 2023

test_wamr.sh: bump wasi-testsuite version #2568

Merged

eloparco mentioned this pull request Sep 20, 2023

proc_exit and traps do not stop thread executing blocking instructions #1910

Closed

wenyongh added a commit to wenyongh/wasm-micro-runtime that referenced this pull request Sep 21, 2023

Merge pull request #815 from bytecodealliance/main

c4279bf

Implement async termination of blocking thread (bytecodealliance#2516)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement async termination of blocking thread #2516

Implement async termination of blocking thread #2516

Uh oh!

yamt commented Aug 30, 2023 •

edited

Loading

Uh oh!

yamt commented Aug 31, 2023

Uh oh!

yamt commented Sep 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wenyongh commented Sep 18, 2023

Uh oh!

eloparco commented Sep 19, 2023

Uh oh!

Uh oh!

wenyongh left a comment

Uh oh!

eloparco commented Sep 19, 2023

Uh oh!

yamt commented Sep 20, 2023

Uh oh!

wenyongh commented Sep 20, 2023

Uh oh!

eloparco commented Sep 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement async termination of blocking thread #2516

Implement async termination of blocking thread #2516

Uh oh!

Conversation

yamt commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yamt commented Aug 31, 2023

Uh oh!

yamt commented Sep 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wenyongh commented Sep 18, 2023

Uh oh!

eloparco commented Sep 19, 2023

Uh oh!

Uh oh!

wenyongh left a comment

Choose a reason for hiding this comment

Uh oh!

eloparco commented Sep 19, 2023

Uh oh!

yamt commented Sep 20, 2023

Uh oh!

wenyongh commented Sep 20, 2023

Uh oh!

eloparco commented Sep 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yamt commented Aug 30, 2023 •

edited

Loading