1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
|
From 36b37559cf63760c5b3e717384963baa866273f3 Mon Sep 17 00:00:00 2001
From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sun, 23 Jan 2022 23:36:44 -0600
Subject: [PATCH] Fix exit regression
Currently vte delays the "child-exited" signal to until after eos has
been received. This is wrong and caused a regression.
vte shouldn't tell client terminals when to exit, if the children has
exited, vte should send the "child-exited" signal. Period.
If the client decides to wait for "eof", that's up to the client
terminal.
On Unix if the child spawns grandchildren, their output will be attached
to the controlling terminal (unless a tool like nohup is used),
therefore the eos won't happen until all the grandchildren have closed
their output.
For example this command will keep producing output after the shell has
exited:
sh -c 'for i in $(seq 1 10); do echo $i; sleep 1; done' & exit
vte assumes an eos will always occur, which is why if a shell spawns any
grandchildren, exiting the shell doesn't close the terminal: vte would
wait for all the grandchildren to exit, which from the point of view of
the user it seems the terminal is hanging forever.
To workaround this issue, a 5-second timer was introduced in 058adf5f
(pty: Fix indefinite wait for EOS after child-exited, 2019-12-01), which
was later reduced to 2 seconds, but this is just a hack.
vte should not be waiting for an eos before sending "child-exited".
In the past vte used to send the "child-exited" immediately, which is
the correct desired behavior, but that changed in commit 7888602c (lib:
Rework child exit and EOF handling, 2019-11-17).
The only correct change in that commit is removing unset_pty() from
child_watch_done() which does allow the code to process remaining data
in the buffers, even from grandchildren that take several seconds to
finish.
Delaying "child-exited" was not needed to fix that problem and
introduced the regression that can be easily triggered by simply doing:
sleep infinity & exit
Decoupling "child-exited" from "eof" gets rid of a lot of unnecessary
complexity, and also eliminates the need for the hacky timer.
With this change even the output from grandchildren processes that last
several seconds is processed correctly:
xfce4-terminal --disable-server --hold \
--command="bash -i -c \"sh -c 'for i in \$(seq 1 10); do echo \$i; sleep 1; done' & exit\""
And of course, the vastly most common case of terminals running shells
that want to exit immediately works correctly.
Fixes #204.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
src/vte.cc | 84 ++--------------------------------------------
src/vteinternal.hh | 8 -----
2 files changed, 2 insertions(+), 90 deletions(-)
diff --git a/src/vte.cc b/src/vte.cc
index 7cc26c48..d37dacc9 100644
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -953,62 +953,6 @@ Terminal::queue_eof()
g_object_unref);
}
-void
-Terminal::emit_child_exited()
-{
- auto const status = m_child_exit_status;
- m_child_exit_status = -1;
-
- if (widget())
- widget()->emit_child_exited(status);
-}
-
-static gboolean
-emit_child_exited_idle_cb(VteTerminal *terminal)
-try
-{
- _vte_terminal_get_impl(terminal)->emit_child_exited();
-
- return G_SOURCE_REMOVE;
-}
-catch (...)
-{
- vte::log_exception();
- return G_SOURCE_REMOVE;
-}
-
-/* Emit a "child-exited" signal on idle, so that if the handler destroys
- * the terminal, we're not deep within terminal code callstack
- */
-void
-Terminal::queue_child_exited()
-{
- _vte_debug_print(VTE_DEBUG_SIGNALS, "Queueing `child-exited'.\n");
- m_child_exited_after_eos_pending = false;
-
- g_idle_add_full(G_PRIORITY_HIGH,
- (GSourceFunc)emit_child_exited_idle_cb,
- g_object_ref(m_terminal),
- g_object_unref);
-}
-
-bool
-Terminal::child_exited_eos_wait_callback()
-{
- /* If we get this callback, there has been some time elapsed
- * after child-exited, but no EOS yet. This happens for example
- * when the primary child started other processes in the background,
- * which inherited the PTY, and thus keep it open, see
- * https://gitlab.gnome.org/GNOME/vte/issues/204
- *
- * Force an EOS.
- */
- if (pty())
- pty_io_read(pty()->fd(), G_IO_HUP);
-
- return false; // don't run again
-}
-
/* Emit an "increase-font-size" signal. */
void
Terminal::emit_increase_font_size()
@@ -3273,20 +3217,8 @@ Terminal::child_watch_done(pid_t pid,
m_pty_pid = -1;
- /* If we still have a PTY, or data to process, defer emitting the signals
- * until we have EOF on the PTY, so that we can process all pending data.
- */
- if (pty() || !m_incoming_queue.empty()) {
- m_child_exit_status = status;
- m_child_exited_after_eos_pending = true;
-
- m_child_exited_eos_wait_timer.schedule_seconds(2); // FIXME: better value?
- } else {
- m_child_exited_after_eos_pending = false;
-
- if (widget())
- widget()->emit_child_exited(status);
- }
+ if (widget())
+ widget()->emit_child_exited(status);
}
static void
@@ -4095,9 +4027,6 @@ out:
chunk->set_sealed();
chunk->set_eos();
- /* Cancel wait timer */
- m_child_exited_eos_wait_timer.abort();
-
/* Need to process the EOS */
if (!is_processing()) {
add_process_timeout(this);
@@ -9976,8 +9905,6 @@ Terminal::unset_pty(bool notify_widget)
disconnect_pty_read();
disconnect_pty_write();
- m_child_exited_eos_wait_timer.abort();
-
/* Clear incoming and outgoing queues */
m_input_bytes = 0;
m_incoming_queue = {};
@@ -10273,19 +10200,12 @@ Terminal::emit_pending_signals()
m_bell_pending = false;
}
- auto const eos = m_eos_pending;
if (m_eos_pending) {
queue_eof();
m_eos_pending = false;
unset_pty();
}
-
- if (m_child_exited_after_eos_pending && eos) {
- /* The signal handler could destroy the terminal, so send the signal on idle */
- queue_child_exited();
- m_child_exited_after_eos_pending = false;
- }
}
void
diff --git a/src/vteinternal.hh b/src/vteinternal.hh
index 0d454649..1636d511 100644
--- a/src/vteinternal.hh
+++ b/src/vteinternal.hh
@@ -291,13 +291,7 @@ public:
guint m_pty_output_source{0};
bool m_pty_input_active{false};
pid_t m_pty_pid{-1}; /* pid of child process */
- int m_child_exit_status{-1}; /* pid's exit status, or -1 */
bool m_eos_pending{false};
- bool m_child_exited_after_eos_pending{false};
- bool child_exited_eos_wait_callback();
- vte::glib::Timer m_child_exited_eos_wait_timer{std::bind(&Terminal::child_exited_eos_wait_callback,
- this),
- "child-exited-eos-wait-timer"};
VteReaper *m_reaper;
/* Queue of chunks of data read from the PTY.
@@ -988,7 +982,6 @@ public:
bool terminate_child () noexcept;
void child_watch_done(pid_t pid,
int status);
- void emit_child_exited();
void im_commit(std::string_view const& str);
void im_preedit_set_active(bool active) noexcept;
@@ -1128,7 +1121,6 @@ public:
void queue_cursor_moved();
void queue_contents_changed();
- void queue_child_exited();
void queue_eof();
#ifdef WITH_A11Y
--
2.39.2
|