search

oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.85k stars 158 forks source link

traps in osh -c don't run when the final command is not a shell builtin #1853

Open deviant opened 8 months ago

deviant commented 8 months ago

For example: osh -c 'trap "echo hi" EXIT; sleep 1' should sleep for a second, and then print "hi" to the console. Instead, it prints nothing. This is due to bug in an optimization in osh -c (where it execs the last command). This bug does not occur when the last command is a builtin (for example: echo ho, true, and, confusingly, command true— prefer $(which true) for debugging this!).

Something else I discovered, and found curious:

$ osh -c 'trap "echo hi" EXIT; $(which true)'
# nothing
$ osh -c 'trap "echo hi" EXIT; $(which true)
'
hi

The optimization is suppressed when there are lines following the final command, even if they're empty! This is presumably not desired behaviour.

I discovered this when testing to see which POSIX shell implementations optimize the behaviour of the -c flag, prompted by https://github.com/kakoune-lsp/kakoune-lsp/pull/728. All other shells I've tested (dash, bash, zsh, ksh, mksh, and oksh) correctly suppress this optimization when a signal is trapped.

andychu commented 8 months ago

Very interesting, thanks for testing! We should be able to fix this

IIRC, the presence of a blank line of semi-colon also affects the optimization in other shells

There is another reason we want to turn it off -- to check the exit code, and print an error message after failure!

Let me think about what the desired behavior is

andychu commented 8 months ago

FWIW the quality page - https://www.oilshell.org/release/0.20.0/quality.html

links to test/syscall

which compares behavior across shells - https://www.oilshell.org/release/0.20.0/more-tests.wwz/syscall/by-code.txt

I found out about this "exec optimization" thing several years ago, and I discovered all POSIX shells do it to some extent

I was surprised because it wasn't documented

At the time, OSH started MORE processes than any shell. So I optimized it to start fewer than almost all shells, with 2 simple rules

I also noticed that the ; matters -- e.g. you can see in that table:

I believe I also found cases in other shells where the newline matters, but they don't appear to be in that table

Anyway at some point I will paste the test case into the harness and see what happens ... that is just background, since yeah AFAIK this isn't documented anywhere

We probably should have shopt --unset exec_optimize in Oils or something.

andychu commented 7 months ago

BTW this bug was mentioned here - https://lobste.rs/s/hru0ib/how_lose_control_your_shell

So there are several things that this noforklast optimization conflicts with

  1. Running EXIT trap
  2. Giving back the terminal in a job control shell -- the zed/zsh bug, and also our own bugs
  3. Printing error messages after a failed command
  4. Tracing of processes and dumping stats -- I started implementing OILS_TRACE_DIR recently and noticed this
andychu commented 4 months ago

Related bug today on help-bash - trap RETURN not run due to noforklast optimization

https://lists.gnu.org/archive/html/help-bash/2024-07/msg00008.html

https://oilshell.zulipchat.com/#narrow/stream/384942-language-design/topic/shopt.20--set.20noforklast