From 5c4ce2fadf23bff6f38155df44b5d6040cf80d26 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 15 Sep 2013 14:05:31 +0000 Subject: [PATCH] Only run one toplevel callback per event loop iteration. This change attempts to reinstate as a universal property something which was sporadically true of the ad-hockery that came before toplevel callbacks: that if there's a _very long_ queue of things to be done through the callback mechanism, the doing of them will be interleaved with re-checks of other event sources, which might (e.g.) cause a flag to be set which makes the next callback decide not to do anything after all. [originally from svn r10040] --- callback.c | 34 ++++++++++++++++++++++++++++++---- putty.h | 7 ++++++- unix/gtkwin.c | 3 ++- unix/uxplink.c | 35 ++++++++++++++++++++--------------- unix/uxsftp.c | 37 ++++++++++++++++++++++--------------- windows/window.c | 22 +++++++++++++--------- windows/winplink.c | 4 +++- windows/winsftp.c | 4 +++- 8 files changed, 99 insertions(+), 47 deletions(-) diff --git a/callback.c b/callback.c index e62fcd7d..c850f6bc 100644 --- a/callback.c +++ b/callback.c @@ -26,10 +26,28 @@ void request_callback_notifications(toplevel_callback_notify_fn_t fn, frontend = fr; } +void stoat_callback(void *ctx) +{ + static int stoat = 0; + if (++stoat % 1000 == 0) + debug(("stoat %d\n", stoat)); + queue_toplevel_callback(stoat_callback, NULL); +} +void queue_stoat(void) +{ + static int stoat = 0; + if (!stoat) { + stoat = 1; + queue_toplevel_callback(stoat_callback, NULL); + } +} + void queue_toplevel_callback(toplevel_callback_fn_t fn, void *ctx) { struct callback *cb; + queue_stoat(); + cb = snew(struct callback); cb->fn = fn; cb->ctx = ctx; @@ -50,19 +68,27 @@ void queue_toplevel_callback(toplevel_callback_fn_t fn, void *ctx) void run_toplevel_callbacks(void) { - while (cbhead) { + queue_stoat(); + if (cbhead) { struct callback *cb = cbhead; /* * Careful ordering here. We call the function _before_ * advancing cbhead (though, of course, we must free cb * _after_ advancing it). This means that if the very last * callback schedules another callback, cbhead does not become - * NULL at any point in this while loop, and so the frontend - * notification function won't be needlessly pestered. + * NULL at any point, and so the frontend notification + * function won't be needlessly pestered. */ cb->fn(cb->ctx); cbhead = cb->next; sfree(cb); + if (!cbhead) + cbtail = NULL; } - cbtail = NULL; +} + +int toplevel_callback_pending(void) +{ + queue_stoat(); + return cbhead != NULL; } diff --git a/putty.h b/putty.h index 7fbb75ec..89140f80 100644 --- a/putty.h +++ b/putty.h @@ -1403,11 +1403,16 @@ void timer_change_notify(unsigned long next); * top-level event loop. However, if a front end doesn't have control * over its own event loop (e.g. because it's using GTK) then it can * instead request notifications when a callback is available, so that - * it knows to ask its delegate event loop to do the same thing. + * it knows to ask its delegate event loop to do the same thing. Also, + * if a front end needs to know whether a callback is pending without + * actually running it (e.g. so as to put a zero timeout on a select() + * call) then it can call toplevel_callback_pending(), which will + * return true if at least one callback is in the queue. */ typedef void (*toplevel_callback_fn_t)(void *ctx); void queue_toplevel_callback(toplevel_callback_fn_t fn, void *ctx); void run_toplevel_callbacks(void); +int toplevel_callback_pending(void); typedef void (*toplevel_callback_notify_fn_t)(void *frontend); void request_callback_notifications(toplevel_callback_notify_fn_t notify, diff --git a/unix/gtkwin.c b/unix/gtkwin.c index 29ac2284..88ce8b55 100644 --- a/unix/gtkwin.c +++ b/unix/gtkwin.c @@ -1422,7 +1422,8 @@ static gint idle_toplevel_callback_func(gpointer data) run_toplevel_callbacks(); } - gtk_idle_remove(inst->toplevel_callback_idle_id); + if (!toplevel_callback_pending()) + gtk_idle_remove(inst->toplevel_callback_idle_id); return TRUE; } diff --git a/unix/uxplink.c b/unix/uxplink.c index 85f91224..ee5cef8a 100644 --- a/unix/uxplink.c +++ b/unix/uxplink.c @@ -979,6 +979,7 @@ int main(int argc, char **argv) int maxfd; int rwx; int ret; + unsigned long next; FD_ZERO(&rset); FD_ZERO(&wset); @@ -1032,12 +1033,17 @@ int main(int argc, char **argv) FD_SET_MAX(fd, maxfd, xset); } - do { - unsigned long next, then; - long ticks; - struct timeval tv, *ptv; + if (toplevel_callback_pending()) { + struct timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 0; + ret = select(maxfd, &rset, &wset, &xset, &tv); + } else if (run_timers(now, &next)) { + do { + unsigned long then; + long ticks; + struct timeval tv; - if (run_timers(now, &next)) { then = now; now = GETTICKCOUNT(); if (now - then > next - then) @@ -1046,16 +1052,15 @@ int main(int argc, char **argv) ticks = next - now; tv.tv_sec = ticks / 1000; tv.tv_usec = ticks % 1000 * 1000; - ptv = &tv; - } else { - ptv = NULL; - } - ret = select(maxfd, &rset, &wset, &xset, ptv); - if (ret == 0) - now = next; - else - now = GETTICKCOUNT(); - } while (ret < 0 && errno == EINTR); + ret = select(maxfd, &rset, &wset, &xset, &tv); + if (ret == 0) + now = next; + else + now = GETTICKCOUNT(); + } while (ret < 0 && errno == EINTR); + } else { + ret = select(maxfd, &rset, &wset, &xset, NULL); + } if (ret < 0) { perror("select"); diff --git a/unix/uxsftp.c b/unix/uxsftp.c index 44e11764..391da021 100644 --- a/unix/uxsftp.c +++ b/unix/uxsftp.c @@ -444,6 +444,7 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok) int i, fdcount, fdsize, *fdlist; int fd, fdstate, rwx, ret, maxfd; unsigned long now = GETTICKCOUNT(); + unsigned long next; fdlist = NULL; fdcount = fdsize = 0; @@ -488,12 +489,19 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok) if (include_stdin) FD_SET_MAX(0, maxfd, rset); - do { - unsigned long next, then; - long ticks; - struct timeval tv, *ptv; + if (toplevel_callback_pending()) { + struct timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 0; + ret = select(maxfd, &rset, &wset, &xset, &tv); + if (ret == 0) + run_toplevel_callbacks(); + } else if (run_timers(now, &next)) { + do { + unsigned long then; + long ticks; + struct timeval tv; - if (run_timers(now, &next)) { then = now; now = GETTICKCOUNT(); if (now - then > next - then) @@ -502,16 +510,15 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok) ticks = next - now; tv.tv_sec = ticks / 1000; tv.tv_usec = ticks % 1000 * 1000; - ptv = &tv; - } else { - ptv = NULL; - } - ret = select(maxfd, &rset, &wset, &xset, ptv); - if (ret == 0) - now = next; - else - now = GETTICKCOUNT(); - } while (ret < 0 && errno != EINTR); + ret = select(maxfd, &rset, &wset, &xset, &tv); + if (ret == 0) + now = next; + else + now = GETTICKCOUNT(); + } while (ret < 0 && errno == EINTR); + } else { + ret = select(maxfd, &rset, &wset, &xset, NULL); + } } while (ret == 0); if (ret < 0) { diff --git a/windows/window.c b/windows/window.c index bc891955..ec6ec3e8 100644 --- a/windows/window.c +++ b/windows/window.c @@ -847,11 +847,20 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) while (1) { HANDLE *handles; int nhandles, n; + DWORD timeout; + + if (toplevel_callback_pending()) { + timeout = 0; + } else { + timeout = INFINITE; + /* The messages seem unreliable; especially if we're being tricky */ + term_set_focus(term, GetForegroundWindow() == hwnd); + } handles = handle_get_events(&nhandles); - n = MsgWaitForMultipleObjects(nhandles, handles, FALSE, INFINITE, - QS_ALLINPUT); + n = MsgWaitForMultipleObjects(nhandles, handles, FALSE, + timeout, QS_ALLINPUT); if ((unsigned)(n - WAIT_OBJECT_0) < (unsigned)nhandles) { handle_got_event(handles[n - WAIT_OBJECT_0]); @@ -859,20 +868,15 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) } else sfree(handles); - run_toplevel_callbacks(); - - while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) { + if (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) { if (msg.message == WM_QUIT) goto finished; /* two-level break */ if (!(IsWindow(logbox) && IsDialogMessage(logbox, &msg))) DispatchMessage(&msg); - - run_toplevel_callbacks(); } - /* The messages seem unreliable; especially if we're being tricky */ - term_set_focus(term, GetForegroundWindow() == hwnd); + run_toplevel_callbacks(); } finished: diff --git a/windows/winplink.c b/windows/winplink.c index 9d29c20b..a618c7ed 100644 --- a/windows/winplink.c +++ b/windows/winplink.c @@ -648,7 +648,9 @@ int main(int argc, char **argv) sending = TRUE; } - if (run_timers(now, &next)) { + if (toplevel_callback_pending()) { + ticks = 0; + } else if (run_timers(now, &next)) { then = now; now = GETTICKCOUNT(); if (now - then > next - then) diff --git a/windows/winsftp.c b/windows/winsftp.c index 23273507..25ac6c94 100644 --- a/windows/winsftp.c +++ b/windows/winsftp.c @@ -493,7 +493,9 @@ int do_eventsel_loop(HANDLE other_event) int skcount; unsigned long now = GETTICKCOUNT(); - if (run_timers(now, &next)) { + if (toplevel_callback_pending()) { + ticks = 0; + } else if (run_timers(now, &next)) { then = now; now = GETTICKCOUNT(); if (now - then > next - then) -- 2.45.2