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

--safe-crash-log-file flag #140

Merged
merged 12 commits into from
Mar 22, 2024
Merged

--safe-crash-log-file flag #140

merged 12 commits into from
Mar 22, 2024

Conversation

d-netto
Copy link
Member

@d-netto d-netto commented Mar 22, 2024

PR Description

Creates a --safe-crash-log-file that is used to redirect crash logs created while we were running the signal handler.

For now it's only x86_64 Linux but shouldn't be that hard to port it to MacOS.

Example usage:

./julia --safe-crash-log-file=/tmp/foobar.txt -e "p = Ptr{Any}(C_NULL); b = unsafe_load(p, 0)"

Which leads to:

cat /tmp/foobar.txt 

[83691] signal (11.1): Segmentation fault
in expression starting at none:1
signal (11) thread (1) unsafe_load at ./pointer.jl:119
signal (11) thread (1) unknown function (ip: 0x7f477566a17f)
signal (11) thread (1) _jl_invoke at /home/ubuntu/julia-RAI/src/gf.c:2895 [inlined]
signal (11) thread (1) ijl_apply_generic at /home/ubuntu/julia-RAI/src/gf.c:3077
signal (11) thread (1) jl_apply at /home/ubuntu/julia-RAI/src/julia.h:1982 [inlined]
signal (11) thread (1) do_call at /home/ubuntu/julia-RAI/src/interpreter.c:126
signal (11) thread (1) eval_value at /home/ubuntu/julia-RAI/src/interpreter.c:223
signal (11) thread (1) eval_stmt_value at /home/ubuntu/julia-RAI/src/interpreter.c:174 [inlined]
signal (11) thread (1) eval_body at /home/ubuntu/julia-RAI/src/interpreter.c:617
signal (11) thread (1) jl_interpret_toplevel_thunk at /home/ubuntu/julia-RAI/src/interpreter.c:775
signal (11) thread (1) jl_toplevel_eval_flex at /home/ubuntu/julia-RAI/src/toplevel.c:934
signal (11) thread (1) jl_toplevel_eval_flex at /home/ubuntu/julia-RAI/src/toplevel.c:877
signal (11) thread (1) jl_toplevel_eval_flex at /home/ubuntu/julia-RAI/src/toplevel.c:877
signal (11) thread (1) ijl_toplevel_eval_in at /home/ubuntu/julia-RAI/src/toplevel.c:985
signal (11) thread (1) eval at ./boot.jl:385 [inlined]
signal (11) thread (1) exec_options at ./client.jl:291
signal (11) thread (1) _start at ./client.jl:552
signal (11) thread (1) jfptr__start_83344 at /home/ubuntu/julia-RAI/usr/lib/julia/sys.so (unknown line)
signal (11) thread (1) _jl_invoke at /home/ubuntu/julia-RAI/src/gf.c:2895 [inlined]
signal (11) thread (1) ijl_apply_generic at /home/ubuntu/julia-RAI/src/gf.c:3077
signal (11) thread (1) jl_apply at /home/ubuntu/julia-RAI/src/julia.h:1982 [inlined]
signal (11) thread (1) true_main at /home/ubuntu/julia-RAI/src/jlapi.c:582
signal (11) thread (1) jl_repl_entrypoint at /home/ubuntu/julia-RAI/src/jlapi.c:731
signal (11) thread (1) main at /home/ubuntu/julia-RAI/cli/loader_exe.c:58
signal (11) thread (1) __libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
signal (11) thread (1) _start at ./julia (unknown line)
Allocations: 2908 (Pool: 2898; Big: 10); GC: 0

Checklist

Requirements for merging:

  • I have opened an issue or PR upstream on JuliaLang/julia: NIL
  • I have removed the port-to-* labels that don't apply.
  • I have opened a PR on raicode to test these changes: NIL

@github-actions github-actions bot added port-to-v1.10 This change should apply to Julia v1.10 builds port-to-master This change should apply to all future Julia builds port-to-v1.9 This change should apply to Julia v1.9 builds labels Mar 22, 2024
Copy link

@li1 li1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have experimented with this on LATEST, changes LGTM through my Julia-noobish eyes. Would be great if @d-netto and/or @kpamnany can take a look at the changes we had to introduce to make this play nice with the OTel ingest.

Copy link
Collaborator

@kpamnany kpamnany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jl_inside_signal_handler() will return 1 for the mutator threads when they're stopped for GC... do they ever call jl_safe_printf while they're waiting?

That's the only question I have about this... are there any other situations where we call jl_safe_printf in signal handler context besides when we're emitting a stack trace.

base/options.jl Show resolved Hide resolved
src/jl_uv.c Outdated Show resolved Hide resolved
src/jl_uv.c Outdated Show resolved Hide resolved
src/jl_uv.c Outdated Show resolved Hide resolved
src/signals-unix.c Outdated Show resolved Hide resolved
src/signals-win.c Outdated Show resolved Hide resolved
@adnan-alhomssi
Copy link
Member

Comment for the future:
The signal handler is in

void jl_critical_error(int sig, int si_code, bt_context_t *context, jl_task_t *ct)
which calls
https://github.com/RelationalAI/julia/blob/v1.10.2%2BRAI/src/stackwalk.c#L658 void jl_print_bt_entry_codeloc(int sig, jl_bt_element_t *bt_entry) JL_NOTSAFEPOINT

We could work up in the stack to emit a single JSON line that includes all backtrace lines if we want to.

Copy link
Collaborator

@kpamnany kpamnany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay. I don't much like the name --safe-crash-log-file, but no need for bike-shedding atm. :)

@d-netto d-netto merged commit 4859e9c into v1.10.2+RAI Mar 22, 2024
5 checks passed
@d-netto d-netto deleted the dcn-safe-crash-log-file branch March 22, 2024 17:06
@d-netto d-netto removed port-to-v1.10 This change should apply to Julia v1.10 builds port-to-master This change should apply to all future Julia builds labels Apr 16, 2024
d-netto added a commit that referenced this pull request Apr 16, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request Apr 23, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request Apr 24, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request Apr 30, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request Apr 30, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request May 2, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request May 9, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request May 19, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request May 26, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request May 28, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
DelveCI pushed a commit that referenced this pull request May 29, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
Drvi pushed a commit that referenced this pull request Jun 7, 2024
* --safe-crash-log-file flag

* Update init.c

* json escape jl_safe_printf when safe crash log file

* add timestamp to json logs

* port it to aarch64 darwin

* fix minor warning

* missing double quote

* Suggestion from code review: make sig_stack_size a const in signals-unix.c

Co-authored-by: Kiran Pamnany <[email protected]>

* Suggestion from code review: make sig_stack size a const in signals-win.c

Co-authored-by: Kiran Pamnany <[email protected]>

* more suggestions from Kiran's review

* more suggestions from review

---------

Co-authored-by: Malte Sandstede <[email protected]>
Co-authored-by: Adnan Alhomssi <[email protected]>
Co-authored-by: Kiran Pamnany <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port-to-v1.9 This change should apply to Julia v1.9 builds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants