Skip to content

Commit 838074a

Browse files
committed
fix(jobs): a bare wait reports like bash's, and listings keep their markers
The arm64 rung kept failing `sleep 0.3 & kill %1; wait; jobs` after the process-group fix, and the cause was an older divergence in what a bare `wait` leaves for `jobs`. bash reaps in its SIGCHLD handler, waits for its jobs in table order, and then retires every dead job except the one `$!` names (mark_dead_jobs_as_notified skips last_asynchronous_pid in a script); a job that died while `wait` was busy with an earlier one stays for `jobs`. hellish's `wait` retired everything it could see -- on the fast side of the race only, which is why arm64 saw it and x86 did not. * wait_all (builtin_wait2.c): drain what is already dead without blocking, wait for the jobs in order marking only the one being waited on as wait's own, mark every dead job but `$!`'s as reported, retire the reported. Interactive shells retire nothing in `wait`: the prompt notifier prints "[1]+ Done" for all of them, as bash does. * `jobs` and the prompt notifier print the whole batch before retiring any job -- removing one mid-listing re-elected the current job and the next line came out "[2] Done" where bash prints "[2]+ Done". * job_reelect follows bash's reset_current: stopped jobs first, then running, never a dead one; with neither, both markers go. tests/wait_reports: 13 cases diffed against bash 5.3.9. The two golden cases that raced `kill %1` straight into `wait` (a coin toss in bash itself) now settle the job first; bg_kill_race_test.py keeps the race covered under contention. Interactive behaviour verified identical to bash on a pty for the same shapes. docker/Dockerfile: the Tumbleweed rung retries `zypper refresh` on a mirror timeout ("Timeout reached Curl error (28)") instead of failing.
1 parent b6f7dad commit 838074a

17 files changed

Lines changed: 416 additions & 121 deletions

‎RELEASE.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,36 @@ shows you how to drive the shell.
88

99
---
1010

11+
## v2.8.10 — *the wait release*
12+
13+
The arm64 rung kept failing one golden case after 2.8.9's process-group
14+
fix, and the reason was a second, older divergence: what a bare `wait`
15+
leaves behind for `jobs`. bash reaps in its SIGCHLD handler, walks its
16+
jobs in table order, and afterwards retires every dead job **except the
17+
one `$!` names** (`mark_dead_jobs_as_notified` skips
18+
`last_asynchronous_pid` in a script). hellish's `wait` retired everything
19+
it could see, so `sleep 0.3 & kill %1; wait; jobs` said nothing where bash
20+
prints `Terminated` — on the fast side of the race only, which is why it
21+
showed up on arm64 and not here.
22+
23+
- **`wait` follows bash's accounting**: already-dead children are drained
24+
first (the handler's work, done late), the jobs are waited for in order
25+
and only the one being waited on counts as `wait`'s own, then every dead
26+
job but `$!`'s is retired. Interactive shells retire nothing in `wait`
27+
— the prompt notifier prints `[1]+ Done` for all of them, as bash does.
28+
- **Markers survive a listing**: `jobs` and the prompt notifier print the
29+
whole batch before retiring any job, so `[1]- Done` `[2]+ Done` comes
30+
out as in bash instead of `[2] Done`; and a re-election after a
31+
removal considers stopped and running jobs only, never a dead one.
32+
- `tests/wait_reports`: 13 bash-diffed cases (dead-before vs dead-during
33+
`wait`, one job vs several, killed vs exited, listings). The two golden
34+
cases that raced `kill %1` straight into `wait` — a coin toss in bash
35+
itself — now settle the job first, and `tests/bg_kill_race_test.py`
36+
keeps the race covered under load.
37+
- The Tumbleweed rung retries `zypper refresh` on a mirror timeout.
38+
39+
---
40+
1141
## v2.8.9 — *the export release*
1242

1343
The first 42 account to load its real `~/.zshrc` inside hellish saw

‎docker/Dockerfile‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ FROM ${BASE}
3434
# "cancel" (exit 4). `zypper dup` is the documented way to bring a Tumbleweed
3535
# system in line with the repositories, downgrades included; it runs only
3636
# when the plain install refuses, so the fast path stays fast.
37+
# The refresh itself is retried: download.opensuse.org answered one CI
38+
# run with "Timeout reached Curl error (28)" on the Oss repo metadata,
39+
# and under set -e that alone was a red rung. Four attempts, 20s apart,
40+
# then the install proceeds with whatever refreshed (it re-fetches what
41+
# it needs) rather than failing the build on a mirror hiccup.
3742
ARG CC=cc
3843

3944
RUN set -eux; \
@@ -54,7 +59,8 @@ RUN set -eux; \
5459
bash python3 diffutils findutils which; \
5560
dnf clean all; \
5661
elif command -v zypper >/dev/null 2>&1; then \
57-
zypper --non-interactive --gpg-auto-import-keys refresh; \
62+
n=0; until zypper --non-interactive --gpg-auto-import-keys refresh; do \
63+
n=$((n + 1)); [ "$n" -ge 4 ] && break; sleep 20; done; \
5864
pkgs="gcc clang make readline-devel ncurses-devel git-core bash \
5965
python3 tar gzip which findutils diffutils"; \
6066
if ! zypper --non-interactive install $pkgs; then \

‎incs/job_control.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ t_job *job_find_id(t_job_table *jt, int id);
7878
t_job *job_find_pgid(t_job_table *jt, pid_t pgid);
7979
void job_remove(t_job_table *jt, int id);
8080
void job_purge_done(t_job_table *jt);
81+
void job_mark_reported(t_job_table *jt, pid_t pid);
82+
void job_purge_reported(t_job_table *jt);
83+
void job_mark_reported_except(t_job_table *jt, pid_t keep);
84+
t_job *job_first_unfinished(t_job_table *jt);
85+
int job_live_id(t_job_table *jt, int not_this);
8186
void job_record_exit(t_job *job, int status);
8287
bool job_finished(const t_job *job);
8388
int job_kill_group(struct s_shell *st, t_job *job, int sig);

‎incs/version.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
/* The single source of truth for the running shell's version. Bumped in step
1717
with the git tag / GitHub release / npm package / docker image. */
18-
# define HELLISH_VERSION "2.8.9"
18+
# define HELLISH_VERSION "2.8.10"
1919

2020
/* Where releases live; used by the `update` builtin and the daily check. */
2121
# define HELLISH_REPO "Univers42/hellish"

‎npm/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "hellish-shell",
3-
"version": "2.8.9",
3+
"version": "2.8.10",
44
"description": "hellish — a fast, POSIX-compliant shell, with horns",
55
"bin": {
66
"hellish": "bin/hellish.js"

‎src/builtins/builtin_jobs.c‎

Lines changed: 106 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,91 +1,106 @@
1-
/* ************************************************************************** */
2-
/* */
3-
/* ::: :::::::: */
4-
/* builtin_jobs.c :+: :+: :+: */
5-
/* +:+ +:+ +:+ */
6-
/* By: dlesieur <dlesieur@student.42.fr> +#+ +:+ +#+ */
7-
/* +#+#+#+#+#+ +#+ */
8-
/* Created: 2026/03/16 00:00:00 by dlesieur #+# #+# */
9-
/* Updated: 2026/03/16 00:00:00 by dlesieur ### ########.fr */
10-
/* */
11-
/* ************************************************************************** */
12-
13-
#include "builtins_private.h"
14-
#include "job_control.h"
15-
16-
/* Scan all option words for -l (long format) and -p (PIDs only). We use
17-
ft_strchr so combined flags like -lp work without writing two branches. */
18-
static void parse_jobs_flags(char **av, int ac, bool *show_pid, bool *long_fmt)
19-
{
20-
int i;
21-
22-
*show_pid = false;
23-
*long_fmt = false;
24-
i = 1;
25-
while (i < ac && av[i][0] == '-')
26-
{
27-
if (ft_strchr(av[i], 'l'))
28-
*long_fmt = true;
29-
if (ft_strchr(av[i], 'p'))
30-
*show_pid = true;
31-
i++;
32-
}
33-
}
34-
35-
/* A finished job that has just been listed is over as far as the table is
36-
concerned: park its exit status in the bg_done ring so `wait` can still
37-
answer for it, then drop the entry so its NUMBER goes back into use. */
38-
static void retire_reported(t_shell *state, t_job *job)
39-
{
40-
job->notified = true;
41-
bg_done_record(state, job->pgid, job->raw_status);
42-
job_remove(&state->job_table, job->id);
43-
}
44-
45-
/* jobs [-l] [-p]: list background (and stopped) jobs. We update statuses
46-
first via job_update_status (which reaps any that have finished since the
47-
last check) so the listing is accurate. -p prints only the process-group
48-
ID, useful for `kill $(jobs -p)`. -l adds the PID column.
49-
50-
Listing a finished job's STATUS retires it, and that is what frees its
51-
number:
52-
bash is back at [1] for the next job where hellish used to climb to [2].
53-
The comment that stood here claimed bash keeps the entry so a later
54-
`wait $!` can still answer -- half right. bash keeps the STATUS, not the
55-
job; retire_reported hands the raw waitpid word to the bg_done ring on
56-
the way out, which is the same trick and is why dropping the entry is
57-
safe.
58-
59-
-p is the exception: it prints pgids, never a status, so bash does not
60-
count it as having reported anything and the job stays.
61-
62-
We walk job NUMBERS (job_next_after) rather than table slots: a reaped
63-
job frees its slot, a later job reuses that slot, and slot order then
64-
prints [1] [4] [3] where bash prints [1] [3] [4]. */
65-
int builtin_jobs(t_shell *state, t_vec argv)
66-
{
67-
t_job_table *jt;
68-
t_job *job;
69-
bool show_pid;
70-
bool long_fmt;
71-
int i;
72-
73-
jt = &state->job_table;
74-
parse_jobs_flags((char **)argv.ctx, (int)argv.len, &show_pid, &long_fmt);
75-
job_update_status(state);
76-
i = job_next_after(jt, 0);
77-
while (i)
78-
{
79-
job = job_find_id(jt, i);
80-
i = job_next_after(jt, i);
81-
if (job_finished(job) && job->notified)
82-
continue ;
83-
if (show_pid)
84-
ft_printf("%d\n", job->pgid);
85-
else
86-
job_print(job, jt->current, jt->previous, long_fmt);
87-
if (job_finished(job) && !show_pid)
88-
retire_reported(state, job);
89-
}
90-
return (0);
91-
}
1+
/* ************************************************************************** */
2+
/* */
3+
/* ::: :::::::: */
4+
/* builtin_jobs.c :+: :+: :+: */
5+
/* +:+ +:+ +:+ */
6+
/* By: dlesieur <dlesieur@student.42.fr> +#+ +:+ +#+ */
7+
/* +#+#+#+#+#+ +#+ */
8+
/* Created: 2026/03/16 00:00:00 by dlesieur #+# #+# */
9+
/* Updated: 2026/03/16 00:00:00 by dlesieur ### ########.fr */
10+
/* */
11+
/* ************************************************************************** */
12+
13+
#include "builtins_private.h"
14+
#include "job_control.h"
15+
16+
/* Scan all option words for -l (long format) and -p (PIDs only). We use
17+
ft_strchr so combined flags like -lp work without writing two branches. */
18+
static void parse_jobs_flags(char **av, int ac, bool *show_pid, bool *long_fmt)
19+
{
20+
int i;
21+
22+
*show_pid = false;
23+
*long_fmt = false;
24+
i = 1;
25+
while (i < ac && av[i][0] == '-')
26+
{
27+
if (ft_strchr(av[i], 'l'))
28+
*long_fmt = true;
29+
if (ft_strchr(av[i], 'p'))
30+
*show_pid = true;
31+
i++;
32+
}
33+
}
34+
35+
/* A finished job that has just been listed is over as far as the table is
36+
concerned: park its exit status in the bg_done ring so `wait` can still
37+
answer for it, and mark it reported. The entry itself is dropped only
38+
once the WHOLE listing is out (job_purge_reported below): removing it
39+
here re-elected the current job mid-listing, and the next line came
40+
out as "[2] Done" where bash prints "[2]+ Done" -- bash prints the
41+
batch with the markers as they stood, then deletes. */
42+
static void retire_reported(t_shell *state, t_job *job)
43+
{
44+
job->notified = true;
45+
bg_done_record(state, job->pgid, job->raw_status);
46+
}
47+
48+
/* One line of the listing: the pgid alone under -p, else the full row;
49+
a finished job is marked reported on the way out (retired after the
50+
whole listing, see retire_reported). */
51+
static void list_one(t_shell *state, t_job *job, bool show_pid, bool long_fmt)
52+
{
53+
t_job_table *jt;
54+
55+
jt = &state->job_table;
56+
if (show_pid)
57+
ft_printf("%d\n", job->pgid);
58+
else
59+
job_print(job, jt->current, jt->previous, long_fmt);
60+
if (job_finished(job) && !show_pid)
61+
retire_reported(state, job);
62+
}
63+
64+
/* jobs [-l] [-p]: list background (and stopped) jobs. We update statuses
65+
first via job_update_status (which reaps any that have finished since the
66+
last check) so the listing is accurate. -p prints only the process-group
67+
ID, useful for `kill $(jobs -p)`. -l adds the PID column.
68+
69+
Listing a finished job's STATUS retires it, and that is what frees its
70+
number:
71+
bash is back at [1] for the next job where hellish used to climb to [2].
72+
The comment that stood here claimed bash keeps the entry so a later
73+
`wait $!` can still answer -- half right. bash keeps the STATUS, not the
74+
job; retire_reported hands the raw waitpid word to the bg_done ring on
75+
the way out, which is the same trick and is why dropping the entry is
76+
safe.
77+
78+
-p is the exception: it prints pgids, never a status, so bash does not
79+
count it as having reported anything and the job stays.
80+
81+
We walk job NUMBERS (job_next_after) rather than table slots: a reaped
82+
job frees its slot, a later job reuses that slot, and slot order then
83+
prints [1] [4] [3] where bash prints [1] [3] [4]. */
84+
int builtin_jobs(t_shell *state, t_vec argv)
85+
{
86+
t_job_table *jt;
87+
t_job *job;
88+
bool show_pid;
89+
bool long_fmt;
90+
int i;
91+
92+
jt = &state->job_table;
93+
parse_jobs_flags((char **)argv.ctx, (int)argv.len, &show_pid, &long_fmt);
94+
job_update_status(state);
95+
i = job_next_after(jt, 0);
96+
while (i)
97+
{
98+
job = job_find_id(jt, i);
99+
i = job_next_after(jt, i);
100+
if (!(job_finished(job) && job->notified))
101+
list_one(state, job, show_pid, long_fmt);
102+
}
103+
if (!show_pid)
104+
job_purge_reported(jt);
105+
return (0);
106+
}

‎src/builtins/builtin_proc.c‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,7 @@ int reaped_job_status(t_shell *state, pid_t pid)
110110
int builtin_wait(t_shell *state, t_vec argv)
111111
{
112112
int status;
113-
int drop;
114113
size_t i;
115-
pid_t pid;
116114

117115
if (argv.len >= 2 && ft_strcmp(((char **)argv.ctx)[1], "-n") == 0)
118116
return (wait_n(state));
@@ -124,14 +122,7 @@ int builtin_wait(t_shell *state, t_vec argv)
124122
status = wait_one(state, ((char **)argv.ctx)[i++]);
125123
return (status);
126124
}
127-
pid = pal_wait_any(state, &status, 0);
128-
while (pid > 0)
129-
{
130-
bg_done_record(state, pid, status);
131-
bg_done_take(state, pid, &drop);
132-
pid = pal_wait_any(state, &status, 0);
133-
}
134-
return (job_purge_done(&state->job_table), 0);
125+
return (wait_all(state));
135126
}
136127

137128
/* builtin_times moved to builtin_proc2.c to keep this file within the

0 commit comments

Comments
 (0)