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

threads: during abrupt thread-exit, cleanup anyways #48223

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Jan 11, 2023

Closes #47590 (pthread_cancel still forbidden though, since async mode will corrupt the process, and synchronously tested is just a slow implementation of a boolean)

Refs #47201 (only deals with thread exit, not other case where this is an issue, like cfunction exit and gc-safe-leave)

May help #46537, by blocking jl_wake_libuv before uv_library_shutdown, and other tweaks to GC mode. For example:

[4011824] signal (6.-6): Aborted
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) uv__async_send at /workspace/srcdir/libuv/src/unix/async.c:198 uv_async_send at /workspace/srcdir/libuv/src/unix/async.c:73 jl_wake_libuv at /data/vtjnash/julia1/src/jl_uv.c:44 [inlined] JL_UV_LOCK at /data/vtjnash/julia1/src/jl_uv.c:64 [inlined] ijl_iolock_begin at /data/vtjnash/julia1/src/jl_uv.c:72 iolock_begin at ./libuv.jl:48 [inlined]
_trywait at ./asyncevent.jl:140
wait at ./asyncevent.jl:155 [inlined]
profile_printing_listener at /data/vtjnash/julia1/usr/share/julia/stdlib/v1.10/Profile/src/Profile.jl:39 jfptr_YY.3_58617 at /data/vtjnash/julia1/usr/lib/julia/sys.so (unknown line) _jl_invoke at /data/vtjnash/julia1/src/gf.c:2665 [inlined] ijl_apply_generic at /data/vtjnash/julia1/src/gf.c:2866 jl_apply at /data/vtjnash/julia1/src/julia.h:1870 [inlined] start_task at /data/vtjnash/julia1/src/task.c:1093 Aborted

Fixes #37400

@vtjnash vtjnash added the domain:multithreading Base.Threads and related functionality label Jan 11, 2023
@vtjnash vtjnash force-pushed the jn/threads-init-exit branch 2 times, most recently from cf11026 to 6034d94 Compare January 11, 2023 16:09
@vtjnash vtjnash requested a review from maleadt January 11, 2023 17:40
@maleadt
Copy link
Member

maleadt commented Jan 12, 2023

Thanks, does indeed fix #47590.

pthread_cancel still forbidden though, since async mode will corrupt the process, and synchronously tested is just a slow implementation of a boolean

So synchronous/deferred pthread_cancel isn't forbidden, and indeed seems to work (this matter when e.g. we don't control the code that manages threads, but we can set our own thread to PTHREAD_CANCEL_DEFERRED).

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jan 12, 2023

deferred is the default, but it would need to be used in concert with PTHREAD_CANCEL_DISABLE too, since it will corrupt the mutex state of any correctly written multi-threaded process, and thus should lead to a guaranteed deadlock.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jan 13, 2023

boo

Dates/conversions                                (5) |         failed at 2023-01-11T13:59:58.377
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-macmini-x64-1.0/build/default-macmini-x64-1-0/julialang/julia-master/julia-6034d94057/share/julia/stdlib/v1.10/Dates/test/conversions.jl:63
  Expression: abs(Dates.now() - now(Dates.UTC)) < Dates.Second(1)
   Evaluated: Dates.Millisecond(1001) < Dates.Second(1)

@gbaraldi
Copy link
Member

Should we make that a bit more robust? :)

Closes #47590 (pthread_cancel still forbidden though, since async mode
will corrupt or deadlock the process, and synchronously tested with
cancelation disabled whenever this is a lock is just a slow
implementation of a boolean)

Refs #47201 (only deals with thread exit, not other case where this is
an issue, like cfunction exit and gc-safe-leave)

May help #46537, by blocking jl_wake_libuv before uv_library_shutdown,
and other tweaks to GC mode. For example, avoiding:

[4011824] signal (6.-6): Aborted
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
uv__async_send at /workspace/srcdir/libuv/src/unix/async.c:198
uv_async_send at /workspace/srcdir/libuv/src/unix/async.c:73
jl_wake_libuv at /data/vtjnash/julia1/src/jl_uv.c:44 [inlined]
JL_UV_LOCK at /data/vtjnash/julia1/src/jl_uv.c:64 [inlined]
ijl_iolock_begin at /data/vtjnash/julia1/src/jl_uv.c:72
iolock_begin at ./libuv.jl:48 [inlined]
_trywait at ./asyncevent.jl:140
wait at ./asyncevent.jl:155 [inlined]
profile_printing_listener at /data/vtjnash/julia1/usr/share/julia/stdlib/v1.10/Profile/src/Profile.jl:39
jfptr_YY.3_58617 at /data/vtjnash/julia1/usr/lib/julia/sys.so (unknown line)
_jl_invoke at /data/vtjnash/julia1/src/gf.c:2665 [inlined]
ijl_apply_generic at /data/vtjnash/julia1/src/gf.c:2866
jl_apply at /data/vtjnash/julia1/src/julia.h:1870 [inlined]
start_task at /data/vtjnash/julia1/src/task.c:1093
Aborted

Fixes #37400
@vtjnash vtjnash merged commit 428d242 into master Jan 14, 2023
@vtjnash vtjnash deleted the jn/threads-init-exit branch January 14, 2023 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:multithreading Base.Threads and related functionality
Projects
None yet
3 participants