X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=blobdiff_plain;f=windows%2Fwinhandl.c;h=e5a5e2d541aea5c00848bd8d314fd33a65f50636;hb=af1460d6e5044a3344aaacd15c91cfdcb58578e7;hp=969880905116e107bcd89509bf71d77e3eb62411;hpb=1b3edafcff9ddcd915a5725df2ac33cdc2a16b96;p=PuTTY.git diff --git a/windows/winhandl.c b/windows/winhandl.c index 96988090..e5a5e2d5 100644 --- a/windows/winhandl.c +++ b/windows/winhandl.c @@ -65,7 +65,7 @@ struct handle_generic { void *privdata; /* for client to remember who they are */ }; -typedef enum { INPUT, OUTPUT, FOREIGN } HandleType; +typedef enum { HT_INPUT, HT_OUTPUT, HT_FOREIGN } HandleType; /* ---------------------------------------------------------------------- * Input threads. @@ -115,7 +115,7 @@ static DWORD WINAPI handle_input_threadfunc(void *param) struct handle_input *ctx = (struct handle_input *) param; OVERLAPPED ovl, *povl; HANDLE oev; - int readret, readlen; + int readret, readlen, finished; if (ctx->flags & HANDLE_FLAG_OVERLAPPED) { povl = &ovl; @@ -165,14 +165,32 @@ static DWORD WINAPI handle_input_threadfunc(void *param) (ctx->flags & HANDLE_FLAG_IGNOREEOF)) continue; + /* + * If we just set ctx->len to 0, that means the read operation + * has returned end-of-file. Telling that to the main thread + * will cause it to set its 'defunct' flag and dispose of the + * handle structure at the next opportunity, in which case we + * mustn't touch ctx at all after the SetEvent. (Hence we do + * even _this_ check before the SetEvent.) + */ + finished = (ctx->len == 0); + SetEvent(ctx->ev_to_main); - if (!ctx->len) + if (finished) break; WaitForSingleObject(ctx->ev_from_main, INFINITE); - if (ctx->done) - break; /* main thread told us to shut down */ + if (ctx->done) { + /* + * The main thread has asked us to shut down. Send back an + * event indicating that we've done so. Hereafter we must + * not touch ctx at all, because the main thread might + * have freed it. + */ + SetEvent(ctx->ev_to_main); + break; + } } if (povl) @@ -278,6 +296,12 @@ static DWORD WINAPI handle_output_threadfunc(void *param) while (1) { WaitForSingleObject(ctx->ev_from_main, INFINITE); if (ctx->done) { + /* + * The main thread has asked us to shut down. Send back an + * event indicating that we've done so. Hereafter we must + * not touch ctx at all, because the main thread might + * have freed it. + */ SetEvent(ctx->ev_to_main); break; } @@ -302,8 +326,16 @@ static DWORD WINAPI handle_output_threadfunc(void *param) } SetEvent(ctx->ev_to_main); - if (!writeret) + if (!writeret) { + /* + * The write operation has suffered an error. Telling that + * to the main thread will cause it to set its 'defunct' + * flag and dispose of the handle structure at the next + * opportunity, so we must not touch ctx at all after + * this. + */ break; + } } if (povl) @@ -379,9 +411,9 @@ static int handle_cmp_evtomain(void *av, void *bv) struct handle *a = (struct handle *)av; struct handle *b = (struct handle *)bv; - if ((unsigned)a->u.g.ev_to_main < (unsigned)b->u.g.ev_to_main) + if ((uintptr_t)a->u.g.ev_to_main < (uintptr_t)b->u.g.ev_to_main) return -1; - else if ((unsigned)a->u.g.ev_to_main > (unsigned)b->u.g.ev_to_main) + else if ((uintptr_t)a->u.g.ev_to_main > (uintptr_t)b->u.g.ev_to_main) return +1; else return 0; @@ -392,9 +424,9 @@ static int handle_find_evtomain(void *av, void *bv) HANDLE *a = (HANDLE *)av; struct handle *b = (struct handle *)bv; - if ((unsigned)*a < (unsigned)b->u.g.ev_to_main) + if ((uintptr_t)*a < (uintptr_t)b->u.g.ev_to_main) return -1; - else if ((unsigned)*a > (unsigned)b->u.g.ev_to_main) + else if ((uintptr_t)*a > (uintptr_t)b->u.g.ev_to_main) return +1; else return 0; @@ -406,7 +438,7 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata, struct handle *h = snew(struct handle); DWORD in_threadid; /* required for Win9x */ - h->type = INPUT; + h->type = HT_INPUT; h->u.i.h = handle; h->u.i.ev_to_main = CreateEvent(NULL, FALSE, FALSE, NULL); h->u.i.ev_from_main = CreateEvent(NULL, FALSE, FALSE, NULL); @@ -434,7 +466,7 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata, struct handle *h = snew(struct handle); DWORD out_threadid; /* required for Win9x */ - h->type = OUTPUT; + h->type = HT_OUTPUT; h->u.o.h = handle; h->u.o.ev_to_main = CreateEvent(NULL, FALSE, FALSE, NULL); h->u.o.ev_from_main = CreateEvent(NULL, FALSE, FALSE, NULL); @@ -463,7 +495,7 @@ struct handle *handle_add_foreign_event(HANDLE event, { struct handle *h = snew(struct handle); - h->type = FOREIGN; + h->type = HT_FOREIGN; h->u.f.h = INVALID_HANDLE_VALUE; h->u.f.ev_to_main = event; h->u.f.ev_from_main = INVALID_HANDLE_VALUE; @@ -484,7 +516,7 @@ struct handle *handle_add_foreign_event(HANDLE event, int handle_write(struct handle *h, const void *data, int len) { - assert(h->type == OUTPUT); + assert(h->type == HT_OUTPUT); assert(h->u.o.outgoingeof == EOF_NO); bufchain_add(&h->u.o.queued_data, data, len); handle_try_output(&h->u.o); @@ -500,7 +532,7 @@ void handle_write_eof(struct handle *h) * bidirectional handle if we're still interested in its incoming * direction! */ - assert(h->type == OUTPUT); + assert(h->type == HT_OUTPUT); if (!h->u.o.outgoingeof == EOF_NO) { h->u.o.outgoingeof = EOF_PENDING; handle_try_output(&h->u.o); @@ -537,7 +569,7 @@ HANDLE *handle_get_events(int *nevents) static void handle_destroy(struct handle *h) { - if (h->type == OUTPUT) + if (h->type == HT_OUTPUT) bufchain_clear(&h->u.o.queued_data); CloseHandle(h->u.g.ev_from_main); CloseHandle(h->u.g.ev_to_main); @@ -547,17 +579,18 @@ static void handle_destroy(struct handle *h) void handle_free(struct handle *h) { - /* - * If the handle is currently busy, we cannot immediately free - * it. Instead we must wait until it's finished its current - * operation, because otherwise the subthread will write to - * invalid memory after we free its context from under it. - */ assert(h && !h->u.g.moribund); - if (h->u.g.busy) { - /* - * Just set the moribund flag, which will be noticed next - * time an operation completes. + if (h->u.g.busy && h->type != HT_FOREIGN) { + /* + * If the handle is currently busy, we cannot immediately free + * it, because its subthread is in the middle of something. + * (Exception: foreign handles don't have a subthread.) + * + * Instead we must wait until it's finished its current + * operation, because otherwise the subthread will write to + * invalid memory after we free its context from under it. So + * we set the moribund flag, which will be noticed next time + * an operation completes. */ h->u.g.moribund = TRUE; } else if (h->u.g.defunct) { @@ -599,10 +632,12 @@ void handle_got_event(HANDLE event) if (h->u.g.moribund) { /* - * A moribund handle is already treated as dead from the - * external user's point of view, so do nothing with the - * actual event. Just signal the thread to die if - * necessary, or destroy the handle if not. + * A moribund handle is one which we have either already + * signalled to die, or are waiting until its current I/O op + * completes to do so. Either way, it's treated as already + * dead from the external user's point of view, so we ignore + * the actual I/O result. We just signal the thread to die if + * we haven't yet done so, or destroy the handle if not. */ if (h->u.g.done) { handle_destroy(h); @@ -617,7 +652,7 @@ void handle_got_event(HANDLE event) switch (h->type) { int backlog; - case INPUT: + case HT_INPUT: h->u.i.busy = FALSE; /* @@ -627,15 +662,15 @@ void handle_got_event(HANDLE event) /* * EOF, or (nearly equivalently) read error. */ - h->u.i.gotdata(h, NULL, -h->u.i.readerr); h->u.i.defunct = TRUE; + h->u.i.gotdata(h, NULL, -h->u.i.readerr); } else { backlog = h->u.i.gotdata(h, h->u.i.buffer, h->u.i.len); handle_throttle(&h->u.i, backlog); } break; - case OUTPUT: + case HT_OUTPUT: h->u.o.busy = FALSE; /* @@ -649,8 +684,8 @@ void handle_got_event(HANDLE event) * and mark the thread as defunct (because the output * thread is terminating by now). */ - h->u.o.sentdata(h, -h->u.o.writeerr); h->u.o.defunct = TRUE; + h->u.o.sentdata(h, -h->u.o.writeerr); } else { bufchain_consume(&h->u.o.queued_data, h->u.o.lenwritten); h->u.o.sentdata(h, bufchain_size(&h->u.o.queued_data)); @@ -658,7 +693,7 @@ void handle_got_event(HANDLE event) } break; - case FOREIGN: + case HT_FOREIGN: /* Just call the callback. */ h->u.f.callback(h->u.f.ctx); break; @@ -667,13 +702,13 @@ void handle_got_event(HANDLE event) void handle_unthrottle(struct handle *h, int backlog) { - assert(h->type == INPUT); + assert(h->type == HT_INPUT); handle_throttle(&h->u.i, backlog); } int handle_backlog(struct handle *h) { - assert(h->type == OUTPUT); + assert(h->type == HT_OUTPUT); return bufchain_size(&h->u.o.queued_data); }