1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-04-12 16:48:06 -05:00

handle_write_eof: delegate CloseHandle back to the client.

When a writable HANDLE is managed by the handle-io.c system, you ask
to send EOF on the handle by calling handle_write_eof. That waits
until all buffered data has been written, and then sends an EOF event
by simply closing the handle.

That is, of course, the only way to send an EOF signal on a handle at
all. And yet, it's a bug, because the handle_output system does not
take ownership of the handle you give it: the client of handle_output
retains ownership, keeps its own copy of the handle, and will expect
to close it itself.

In most cases, the extra close will harmlessly fail, and return
ERROR_INVALID_HANDLE (which the caller didn't notice anyway). But if
you're unlucky, in conditions of frantic handle opening and closing
(e.g. with a lot of separate named-pipe-style agent forwarding
connections being constantly set up and torn down), the handle value
might have been reused between the two closes, so that the second
CloseHandle closes an unrelated handle belonging to some other part of
the program.

We can't fix this by giving handle_output permanent ownership of the
handle, because it really _is_ necessary for copies of it to survive
elsewhere: in particular, for a bidirectional file such as a serial
port or named pipe, the reading side also needs a copy of the same
handle! And yet, we can't replace the handle_write_eof call in the
client with a direct CloseHandle, because that won't wait until
buffered output has been drained.

The solution is that the client still calls handle_write_eof to
register that it _wants_ an EOF sent; the handle_output system will
wait until it's ready, but then, instead of calling CloseHandle, it
will ask its _client_ to close the handle, by calling the provided
'sentdata' callback with the new 'close' flag set to true. And then
the client can not only close the handle, but do whatever else it
needs to do to record that that has been done.
This commit is contained in:
Simon Tatham 2021-09-30 19:16:20 +01:00
parent 1541974564
commit dde6590040
6 changed files with 30 additions and 10 deletions

View File

@ -120,7 +120,8 @@ static size_t conpty_gotdata(
}
}
static void conpty_sentdata(struct handle *h, size_t new_backlog, int err)
static void conpty_sentdata(struct handle *h, size_t new_backlog, int err,
bool close)
{
ConPTY *conpty = (ConPTY *)handle_get_privdata(h);
if (err) {

View File

@ -284,6 +284,7 @@ struct handle_output {
* drops.
*/
handle_outputfn_t sentdata;
struct handle *sentdata_param;
};
static DWORD WINAPI handle_output_threadfunc(void *param)
@ -361,7 +362,7 @@ static void handle_try_output(struct handle_output *ctx)
ctx->busy = true;
} else if (!ctx->busy && bufchain_size(&ctx->queued_data) == 0 &&
ctx->outgoingeof == EOF_PENDING) {
CloseHandle(ctx->h);
ctx->sentdata(ctx->sentdata_param, 0, 0, true);
ctx->h = INVALID_HANDLE_VALUE;
ctx->outgoingeof = EOF_SENT;
}
@ -507,6 +508,7 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
bufchain_init(&h->u.o.queued_data);
h->u.o.outgoingeof = EOF_NO;
h->u.o.sentdata = sentdata;
h->u.o.sentdata_param = h;
h->u.o.flags = flags;
ensure_ready_event_setup();
@ -644,11 +646,11 @@ static void handle_ready(struct handle *h)
* thread is terminating by now).
*/
h->u.o.defunct = true;
h->u.o.sentdata(h, 0, h->u.o.writeerr);
h->u.o.sentdata(h, 0, h->u.o.writeerr, false);
} else {
bufchain_consume(&h->u.o.queued_data, h->u.o.lenwritten);
noise_ultralight(NOISE_SOURCE_IOLEN, h->u.o.lenwritten);
h->u.o.sentdata(h, bufchain_size(&h->u.o.queued_data), 0);
h->u.o.sentdata(h, bufchain_size(&h->u.o.queued_data), 0, false);
handle_try_output(&h->u.o);
}
break;

View File

@ -93,10 +93,19 @@ static size_t handle_stderr(
return 0;
}
static void handle_sentdata(struct handle *h, size_t new_backlog, int err)
static void handle_sentdata(struct handle *h, size_t new_backlog, int err,
bool close)
{
HandleSocket *hs = (HandleSocket *)handle_get_privdata(h);
if (close) {
if (hs->send_H != INVALID_HANDLE_VALUE)
CloseHandle(hs->send_H);
if (hs->recv_H != INVALID_HANDLE_VALUE && hs->recv_H != hs->send_H)
CloseHandle(hs->recv_H);
hs->send_H = hs->recv_H = INVALID_HANDLE_VALUE;
}
if (err) {
plug_closing(hs->plug, win_strerror(err), err, 0);
return;
@ -125,8 +134,9 @@ static void sk_handle_close(Socket *s)
handle_free(hs->send_h);
handle_free(hs->recv_h);
CloseHandle(hs->send_H);
if (hs->recv_H != hs->send_H)
if (hs->send_H != INVALID_HANDLE_VALUE)
CloseHandle(hs->send_H);
if (hs->recv_H != INVALID_HANDLE_VALUE && hs->recv_H != hs->send_H)
CloseHandle(hs->recv_H);
bufchain_clear(&hs->inputdata);

View File

@ -615,7 +615,7 @@ struct handle;
typedef size_t (*handle_inputfn_t)(
struct handle *h, const void *data, size_t len, int err);
typedef void (*handle_outputfn_t)(
struct handle *h, size_t new_backlog, int err);
struct handle *h, size_t new_backlog, int err, bool close);
struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
void *privdata, int flags);
struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,

View File

@ -216,8 +216,14 @@ size_t stdin_gotdata(struct handle *h, const void *data, size_t len, int err)
return 0;
}
void stdouterr_sent(struct handle *h, size_t new_backlog, int err)
void stdouterr_sent(struct handle *h, size_t new_backlog, int err, bool close)
{
if (close) {
CloseHandle(outhandle);
CloseHandle(errhandle);
outhandle = errhandle = INVALID_HANDLE_VALUE;
}
if (err) {
char buf[4096];
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, err, 0,

View File

@ -73,7 +73,8 @@ static size_t serial_gotdata(
}
}
static void serial_sentdata(struct handle *h, size_t new_backlog, int err)
static void serial_sentdata(struct handle *h, size_t new_backlog, int err,
bool close)
{
Serial *serial = (Serial *)handle_get_privdata(h);
if (err) {