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

Shell: Some backgrounding (yes, again) fixes, and some other random fixes #6021

Merged
merged 7 commits into from
Mar 31, 2021

Conversation

alimpfard
Copy link
Member

@alimpfard alimpfard commented Mar 30, 2021

  • Less #if *_DEBUG
  • SIGCHLD when child "stopped" state changes, + handling that
  • A "PROMPT" function, to maybe try and move away from cryptic yet-more-escapes approach

The last one is too slow to be usable, so this PR is gonna remain a draft for now, I'll split the unrelated fixes if it takes too long.


Profiling 10 iterations of running PROMPT says that about 60% of the runtime is spent in syscalls, with about half of that being just fork() and mmap().
For reference, running the same PROMPT function in linux (with Lagom/shell) takes about 10ms, so there's quite a big room for improvement.

Any clues about why fork() is taking so long (and how to fix it) would be much appreciated, since I'm a complete and total n00b at dealing with this sort of thing.

-> #6043

This is done also by linux (signal.c:936 in v5.11) at least.
It's a pretty handy notification that allows the parent process to skip
going through a `waitpid` and guesswork to figure out the current state
of a child process.
This fixes `fg` and `bg` causing the shell to go into an infinite loop
of trying to `waitpid` until some current job changes state.

a.k.a. "Fix Shell backgrounding, yet again!" :P
This can happen if the process is stopped and continued at a later time.
This makes commands like `foo 2>&1 | bar` behave as expected (which is
to pipe both stdout and stderr of `foo` to stdin of `bar`).
Previously, this would've piped stderr of `foo` into stdout, and the
stdout of `foo` into the stdin of `bar`.
Otherwise it'll have some random value from the stack, and the kernel
will not bother setting it to zero.
Also add a debug print and tweak the FIXME message.
@alimpfard alimpfard changed the title Shell: Some backgrounding (yes, again) fixes, some other random fixes, and one PROMPT for fun Shell: Some backgrounding (yes, again) fixes, and some other random fixes Mar 31, 2021
@alimpfard alimpfard marked this pull request as ready for review March 31, 2021 17:33
@awesomekling awesomekling merged commit cbd62c4 into SerenityOS:master Mar 31, 2021
@alimpfard alimpfard deleted the shell-fun branch December 23, 2021 08:50
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.

2 participants