Skip to content

Commit ce4ae52

Browse files
committed
Fix CTRL-C on a program launched with rlimit and add --foreground option
To correct CTRL-C, we stop setting rlimit as the foreground program and ignore the SIGTTOU signal. However, this does not allow rlimit to be used with an interactive terminal. To solve this problem, we introduce the --foreground option. IT-465
1 parent dcd1db6 commit ce4ae52

File tree

7 files changed

+156
-16
lines changed

7 files changed

+156
-16
lines changed

src/e3/os/data/rlimit-aarch64-linux

-40 Bytes
Binary file not shown.

src/e3/os/data/rlimit-ppc-linux

59.7 KB
Binary file not shown.

src/e3/os/data/rlimit-x86-linux

104 Bytes
Binary file not shown.

src/e3/os/data/rlimit-x86_64-linux

-104 Bytes
Binary file not shown.

tests/tests_e3/os/process/main_test.py

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import subprocess
44
import textwrap
55
import time
6+
import signal
67

78
import e3.fs
89
import e3.os.fs
@@ -115,6 +116,123 @@ def run_test():
115116
e.restore()
116117

117118

119+
@pytest.mark.skipif(sys.platform == "win32", reason="A linux test")
120+
def test_rlimit_ctrl_c():
121+
"""Test rlimit CTRL-C.
122+
123+
When a parent process launched two or more child processes using rlimit, the CTRL-C
124+
command no longer worked.
125+
126+
This was because when an rlimit process was launched, it became the foreground
127+
process.
128+
129+
However, when the foreground process was killed, it left the parent process without
130+
a foreground, so CTRL-C was ignored.
131+
132+
Examples:
133+
Example 1: Spawn 1 rlimit child process:
134+
python (Parent process)
135+
|
136+
-> rlimit (Child process / foreground process)
137+
138+
A CTRL-C appear:
139+
The child process in the foreground was killed, leaving the parent
140+
process alone with no foreground process but no child. Due to our
141+
usage, this posed no known problems. However, leaving the parent
142+
process without a foreground can cause unexpected results.
143+
144+
Example 2: Spawn 2 rlimit child process:
145+
python (Parent process)
146+
|
147+
-> rlimit (Child process) ==> This process was the foreground process as
148+
| long as no other rlimit process was running.
149+
| In this example, we have 2 rlimit processes,
150+
| so this process is not in the foreground.
151+
|
152+
-> rlimit (Child process / foreground process)
153+
154+
A CTRL-C appear:
155+
The foreground process has been killed, leaving no foreground process.
156+
Signals were no longer propagated, so CTRL-C did nothing.
157+
"""
158+
try:
159+
from ptyprocess import PtyProcess
160+
except ImportError:
161+
raise ImportError("ptyprocess is needed to run this tests") from None
162+
163+
# Only a linux test
164+
assert sys.platform.startswith("linux"), "This test make sens only on linux"
165+
166+
script_to_run = """
167+
from __future__ import annotations
168+
169+
import sys
170+
from e3.os.process import Run
171+
172+
p2 = Run(["sleep", "100"], timeout=30, bg=True)
173+
p1 = Run(["sleep", "10"], timeout=1)
174+
# CTRL-C is now blocking
175+
p2.wait()
176+
"""
177+
178+
with open("tmp-test_rlimic_ctrl_c.py", "w") as f:
179+
f.write(script_to_run)
180+
181+
start = time.perf_counter()
182+
p = PtyProcess.spawn([sys.executable, "tmp-test_rlimic_ctrl_c.py"])
183+
time.sleep(5)
184+
p.sendintr()
185+
# !!! Warning:
186+
# if the script_to_run write something on stdout, this will wait forever.
187+
p.wait()
188+
end = time.perf_counter()
189+
assert int(end - start) < 30, f"CTRL-C failed: take {int(end - start)} seconds"
190+
191+
192+
@pytest.mark.skipif(sys.platform == "win32", reason="A linux test")
193+
def test_rlimit_foreground_option():
194+
"""Test rlimit --foreground.
195+
196+
Test if we can read/write from an interactive terminal using rlimit --foreground.
197+
"""
198+
try:
199+
from ptyprocess import PtyProcess
200+
except ImportError:
201+
raise ImportError("ptyprocess is needed to run this tests") from None
202+
203+
# Only a linux test
204+
assert sys.platform.startswith("linux"), "This test make sens only on linux"
205+
206+
# Test with --foreground
207+
os.environ["PS1"] = "$ "
208+
p = PtyProcess.spawn(
209+
[e3.os.process.get_rlimit(), "--foreground", "30", "bash", "--norc", "-i"],
210+
env=os.environ,
211+
echo=False,
212+
)
213+
p.write(b"echo 'test rlimit --foreground'\n", flush=True)
214+
time.sleep(2)
215+
assert p.read() == b"$ test rlimit --foreground\r\n$ "
216+
# Ensure that the process is killed
217+
p.kill(signal.SIGKILL)
218+
219+
# Test without foreground (Should failed)
220+
p = PtyProcess.spawn(
221+
[e3.os.process.get_rlimit(), "30", "bash", "--norc", "-i"],
222+
env=os.environ,
223+
echo=False,
224+
)
225+
p.write(b"echo 'test rlimit (no --foreground)'\n", flush=True)
226+
time.sleep(2)
227+
with pytest.raises(EOFError):
228+
# The echo command should not be executed by bash. So p.read() will raise an
229+
# EOFError. And if not, we will raise an Exception because this is not
230+
# what we want.
231+
p.read()
232+
# Ensure that the process is killed
233+
p.kill(signal.SIGKILL)
234+
235+
118236
def test_not_found():
119237
with pytest.raises(OSError) as err:
120238
e3.os.process.Run(["e3-bin-not-found"])

tools/rlimit/rlimit.c

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@
2424
Usage:
2525
rlimit seconds command [args] */
2626

27+
#define _XOPEN_SOURCE 500 /* For setpgid */
2728
#include <sys/types.h>
2829
#include <unistd.h>
2930
#include <stdio.h>
3031
#include <stdlib.h>
3132
#include <signal.h>
33+
#include <string.h>
3234
#include <sys/types.h>
3335
#include <sys/wait.h>
3436
#include <errno.h>
@@ -37,11 +39,11 @@ void
3739
usage (void)
3840
{
3941
printf ("Usage:\n");
40-
printf (" rlimit seconds command [args]\n");
42+
printf (" rlimit [--foreground] seconds command [args]\n");
4143
exit (1);
4244
}
4345

44-
46+
static int foreground = 0;
4547
static int pid = 0;
4648
/* pid of child (controlled) process */
4749

@@ -72,6 +74,8 @@ reapchild (int nsig)
7274
{
7375
int delay;
7476

77+
(void)nsig;
78+
7579
if (pid > 0) {
7680
int rc;
7781

@@ -125,16 +129,26 @@ reapchild (int nsig)
125129
int
126130
main (int argc, char **argv)
127131
{
132+
int begin;
128133
sigset_t block_cld;
129134

130135
/* we need at least 3 args */
131136
if (argc < 3)
132137
usage ();
133138

139+
begin = 2;
140+
141+
if (strcmp(argv[1], "--foreground") == 0) {
142+
++foreground;
143+
++begin;
144+
}
145+
else if (*argv[1] == '-')
146+
usage();
147+
134148
/* argv[0] = .../rlimit
135-
argv[1] = seconds
136-
argv[2] = command
137-
argv[3] = args */
149+
argv[begin - 1] = seconds
150+
argv[begin] = command
151+
argv[begin + 1] = args */
138152

139153
signal (SIGTERM, terminate_group);
140154
signal (SIGINT, terminate_group);
@@ -179,9 +193,9 @@ main (int argc, char **argv)
179193
sigprocmask(SIG_UNBLOCK, &block_cld, NULL);
180194

181195
/* child exec the command in a new process group */
182-
if (setpgid (0, 0)) {
183-
perror ("setpgid");
184-
exit (4);
196+
if (setpgid (0, 0) == -1) {
197+
perror("setpgid");
198+
exit(4);
185199
}
186200

187201
#if defined (__APPLE__)
@@ -202,30 +216,37 @@ main (int argc, char **argv)
202216
/* ignore SIGTTOU so that tcsetpgrp call does not block. */
203217
signal (SIGTTOU, SIG_IGN);
204218

205-
/* Set child process as foreground process group so that
206-
* children can read from the terminal if needed. Note that
207-
* we ignore any error here because stdin might not be a terminal.
219+
/* If run with --foreground process group, rlimit must be able to
220+
* read and write from a tty (not only read => STDIN is a tty).
221+
*
208222
* If only stdout is a terminal there are no issues with not
209223
* being the foreground process as most terminals are configured
210224
* with TOSTOP off (so SIGTTOU is only emited in case of terminal
211225
* settings change.
212226
*/
213-
tcsetpgrp(0, getpgid(getpid()));
227+
if (foreground && tcsetpgrp(0, getpgid(getpid())) == -1) {
228+
perror("tcsetpgrp");
229+
exit(4);
230+
}
214231

215232
/* Restore SIGTTIN and SIGTTOU signal to their original value
216233
* in order not to impact children processes behaviour. */
217234
signal (SIGTTIN, SIG_DFL);
218-
signal (SIGTTOU, SIG_DFL);
219-
execvp ((const char *) argv[2], (char *const *) &argv[2]);
220-
fprintf (stderr, "rlimit: could not run \"%s\": ", argv[2]);
235+
236+
/* When we are not in the foreground, we ignore SIGTTOU so that child process can
237+
* write. */
238+
if (foreground)
239+
signal (SIGTTOU, SIG_DFL);
240+
execvp ((const char *) argv[begin], (char *const *) &argv[begin]);
241+
fprintf (stderr, "rlimit: could not run \"%s\": ", argv[begin]);
221242
perror ("execvp");
222243
exit (5);
223244

224245
default: {
225246
/* parent sleeps
226247
wake up when the sleep call returns or when
227248
SIGCHLD is received */
228-
int timeout = atoi (argv[1]);
249+
int timeout = atoi (argv[begin - 1]);
229250
int seconds = timeout;
230251

231252
/* At this stage rlimit might not be anymore the foreground process.

tox.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ deps =
66
xdist: pytest-xdist[psutil]
77
# ??? needs to be added as a dep of e3-core
88
requests-cache
9+
ptyprocess;platform_system == "Linux"
910
cov: pytest-cov
1011
cov: coverage
1112

0 commit comments

Comments
 (0)