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
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
|
From: =?utf-8?b?0L3QsNCx?= <nabijaczleweli@nabijaczleweli.xyz>
Date: Tue, 12 Apr 2022 16:25:14 +0200
Subject: [PATCH 1/2] lib/pty: Put master PTY into non-blocking mode and
buffer its output to avoid deadlock
If we filled the script->child buffer before the child had a chance to read any
input, we'd sleep forever in write_all(pty->master), and the child would sleep
forever in write(1<pty->slave>)
By putting the master PTY in non-blocking mode, we can poll(pty->master,
POLLOUT) and keep supplying more data as the child reads from the buffer
Fixes Debian bug #1003095
Signed-off-by: Karel Zak <kzak@redhat.com>
---
include/pty-session.h | 7 +++
lib/pty-session.c | 126 +++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 110 insertions(+), 23 deletions(-)
diff --git a/include/pty-session.h b/include/pty-session.h
index 09eff43..7176d6b 100644
--- a/include/pty-session.h
+++ b/include/pty-session.h
@@ -81,6 +81,13 @@ struct ul_pty {
struct timeval next_callback_time;
+ struct ul_pty_child_buffer {
+ struct ul_pty_child_buffer *next;
+ char buf[BUFSIZ];
+ size_t size, cursor;
+ unsigned int final_input:1; /* drain child before writing */
+ } *child_buffer_head, *child_buffer_tail, *free_buffers;
+
unsigned int isterm:1, /* is stdin terminal? */
slave_echo:1; /* keep ECHO on pty slave */
};
diff --git a/lib/pty-session.c b/lib/pty-session.c
index 6f038e1..880f08d 100644
--- a/lib/pty-session.c
+++ b/lib/pty-session.c
@@ -69,6 +69,15 @@ struct ul_pty *ul_new_pty(int is_stdin_tty)
void ul_free_pty(struct ul_pty *pty)
{
+ struct ul_pty_child_buffer *hd;
+ while((hd = pty->child_buffer_head)) {
+ pty->child_buffer_head = hd->next;
+ free(hd);
+ }
+ while((hd = pty->free_buffers)) {
+ pty->free_buffers = hd->next;
+ free(hd);
+ }
free(pty);
}
@@ -182,7 +191,7 @@ int ul_pty_setup(struct ul_pty *pty)
cfmakeraw(&attrs);
tcsetattr(STDIN_FILENO, TCSANOW, &attrs);
} else {
- DBG(SETUP, ul_debugobj(pty, "create for non-terminal"));
+ DBG(SETUP, ul_debugobj(pty, "create for non-terminal"));
rc = openpty(&pty->master, &pty->slave, NULL, NULL, NULL);
if (rc)
@@ -198,6 +207,8 @@ int ul_pty_setup(struct ul_pty *pty)
tcsetattr(pty->slave, TCSANOW, &attrs);
}
+ fcntl(pty->master, F_SETFL, O_NONBLOCK);
+
sigfillset(&ourset);
if (sigprocmask(SIG_BLOCK, &ourset, NULL)) {
rc = -errno;
@@ -290,9 +301,27 @@ static int write_output(char *obuf, ssize_t bytes)
return 0;
}
-static int write_to_child(struct ul_pty *pty, char *buf, size_t bufsz)
+static int schedule_child_write(struct ul_pty *pty, char *buf, size_t bufsz, int final)
{
- return write_all(pty->master, buf, bufsz);
+ struct ul_pty_child_buffer *stash;
+ if (pty->free_buffers) {
+ stash = pty->free_buffers;
+ pty->free_buffers = stash->next;
+ memset(stash, 0, sizeof(*stash));
+ } else
+ stash = calloc(1, sizeof(*stash));
+ if (!stash)
+ return -1;
+
+ memcpy(stash->buf, buf, bufsz);
+ stash->size = bufsz;
+ stash->final_input = final ? 1 : 0;
+
+ if (pty->child_buffer_head)
+ pty->child_buffer_tail = pty->child_buffer_tail->next = stash;
+ else
+ pty->child_buffer_head = pty->child_buffer_tail = stash;
+ return 0;
}
/*
@@ -311,16 +340,13 @@ static int write_to_child(struct ul_pty *pty, char *buf, size_t bufsz)
* maintains master+slave tty stuff within the session. Use pipe to write to
* pty and assume non-interactive (tee-like) behavior is NOT well supported.
*/
-void ul_pty_write_eof_to_child(struct ul_pty *pty)
+static void drain_child_buffers(struct ul_pty *pty)
{
unsigned int tries = 0;
- struct pollfd fds[] = {
- { .fd = pty->slave, .events = POLLIN }
- };
- char c = DEF_EOF;
+ struct pollfd fd = { .fd = pty->slave, .events = POLLIN };
DBG(IO, ul_debugobj(pty, " waiting for empty slave"));
- while (poll(fds, 1, 10) == 1 && tries < 8) {
+ while (poll(&fd, 1, 10) == 1 && tries < 8) {
DBG(IO, ul_debugobj(pty, " slave is not empty"));
xusleep(250000);
tries++;
@@ -329,7 +355,53 @@ void ul_pty_write_eof_to_child(struct ul_pty *pty)
DBG(IO, ul_debugobj(pty, " slave is empty now"));
DBG(IO, ul_debugobj(pty, " sending EOF to master"));
- write_to_child(pty, &c, sizeof(char));
+}
+
+static int flush_child_buffers(struct ul_pty *pty, int *anything)
+{
+ int ret = 0, any = 0;
+ while (pty->child_buffer_head) {
+ struct ul_pty_child_buffer *hd = pty->child_buffer_head;
+
+ if(hd->final_input)
+ drain_child_buffers(pty);
+
+ DBG(IO, ul_debugobj(hd, " stdin --> master trying %zu bytes", hd->size - hd->cursor));
+ ssize_t ret = write(pty->master, hd->buf + hd->cursor, hd->size - hd->cursor);
+ if (ret == -1) {
+ DBG(IO, ul_debugobj(hd, " EAGAIN"));
+ if (!(errno == EINTR || errno == EAGAIN))
+ ret = -errno;
+ goto out;
+ }
+ DBG(IO, ul_debugobj(hd, " wrote %zd", ret));
+ any = 1;
+ hd->cursor += ret;
+ if (hd->cursor == hd->size) {
+ pty->child_buffer_head = hd->next;
+ if(!hd->next)
+ pty->child_buffer_tail = NULL;
+
+ hd->next = pty->free_buffers;
+ pty->free_buffers = hd;
+ }
+ }
+
+out:
+ /* without sync write_output() will write both input &
+ * shell output that looks like double echoing */
+ if (any)
+ fdatasync(pty->master);
+
+ if (anything)
+ *anything = any;
+ return ret;
+}
+
+void ul_pty_write_eof_to_child(struct ul_pty *pty)
+{
+ char c = DEF_EOF;
+ schedule_child_write(pty, &c, sizeof(char), 1);
}
static int mainloop_callback(struct ul_pty *pty)
@@ -362,7 +434,7 @@ static int handle_io(struct ul_pty *pty, int fd, int *eof)
/* read from active FD */
bytes = read(fd, buf, sizeof(buf));
sigprocmask(SIG_BLOCK, &set, NULL);
- if (bytes < 0) {
+ if (bytes == -1) {
if (errno == EAGAIN || errno == EINTR)
return 0;
return -errno;
@@ -375,15 +447,11 @@ static int handle_io(struct ul_pty *pty, int fd, int *eof)
/* from stdin (user) to command */
if (fd == STDIN_FILENO) {
- DBG(IO, ul_debugobj(pty, " stdin --> master %zd bytes", bytes));
+ DBG(IO, ul_debugobj(pty, " stdin --> master %zd bytes queued", bytes));
- if (write_to_child(pty, buf, bytes))
+ if (schedule_child_write(pty, buf, bytes, 0))
return -errno;
- /* without sync write_output() will write both input &
- * shell output that looks like double echoing */
- fdatasync(pty->master);
-
/* from command (master) to stdout */
} else if (fd == pty->master) {
DBG(IO, ul_debugobj(pty, " master --> stdout %zd bytes", bytes));
@@ -567,6 +635,11 @@ int ul_pty_proxy_master(struct ul_pty *pty)
} else
timeout = pty->poll_timeout;
+ if (pty->child_buffer_head)
+ pfd[POLLFD_MASTER].events |= POLLOUT;
+ else
+ pfd[POLLFD_MASTER].events &= ~POLLOUT;
+
/* wait for input, signal or timeout */
DBG(IO, ul_debugobj(pty, "calling poll() [timeout=%dms]", timeout));
ret = poll(pfd, ARRAY_SIZE(pfd), timeout);
@@ -600,23 +673,30 @@ int ul_pty_proxy_master(struct ul_pty *pty)
if (pfd[i].revents == 0)
continue;
- DBG(IO, ul_debugobj(pty, " active pfd[%s].fd=%d %s %s %s %s",
+ DBG(IO, ul_debugobj(pty, " active pfd[%s].fd=%d %s %s %s %s %s",
i == POLLFD_STDIN ? "stdin" :
i == POLLFD_MASTER ? "master" :
i == POLLFD_SIGNAL ? "signal" : "???",
pfd[i].fd,
pfd[i].revents & POLLIN ? "POLLIN" : "",
+ pfd[i].revents & POLLOUT ? "POLLOUT" : "",
pfd[i].revents & POLLHUP ? "POLLHUP" : "",
pfd[i].revents & POLLERR ? "POLLERR" : "",
pfd[i].revents & POLLNVAL ? "POLLNVAL" : ""));
if (i == POLLFD_SIGNAL)
rc = handle_signal(pty, pfd[i].fd);
- else if (pfd[i].revents & POLLIN)
- rc = handle_io(pty, pfd[i].fd, &eof); /* data */
+ else {
+ if (pfd[i].revents & POLLIN)
+ rc = handle_io(pty, pfd[i].fd, &eof); /* data */
+ if (pfd[i].revents & POLLOUT) /* i == POLLFD_MASTER */
+ rc = flush_child_buffers(pty, NULL);
+ }
if (rc) {
ul_pty_write_eof_to_child(pty);
+ for (int anything = 1; anything;)
+ flush_child_buffers(pty, &anything);
break;
}
@@ -631,11 +711,11 @@ int ul_pty_proxy_master(struct ul_pty *pty)
*/
if ((pfd[i].revents & POLLHUP) || (pfd[i].revents & POLLNVAL) || eof) {
DBG(IO, ul_debugobj(pty, " ignore FD"));
- pfd[i].fd = -1;
if (i == POLLFD_STDIN) {
+ pfd[i].fd = -1;
ul_pty_write_eof_to_child(pty);
- DBG(IO, ul_debugobj(pty, " ignore STDIN"));
- }
+ } else /* i == POLLFD_MASTER */
+ pfd[i].revents &= ~POLLIN;
}
}
if (rc)
|