1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

Fix startup hang in Unix file transfer tools.

This seems to be a knock-on effect of my recent reworking of the SSH
code to be based around queues and callbacks. The loop iteration
function in uxsftp.c (ssh_sftp_do_select) would keep going round its
select loop until something had happened on one of its file
descriptors, and then return to the caller in the assumption that the
resulting data might have triggered whatever condition the caller was
waiting for - and if not, then the caller checks, finds nothing
interesting has happened, and resumes looping with no harm done.

But now, when something happens on an fd, it doesn't _synchronously_
trigger the follow-up condition PSFTP was waiting for (which, at
startup time, happens to be back->sendok() starting to return TRUE).
Instead, it schedules a callback, which will schedule a callback,
which ... ends up setting that flag. But by that time, the loop
function has already returned, the caller has found nothing
interesting and resumed looping, and _now_ the interesting thing
happens but it's too late because ssh_sftp_do_select will wait until
the next file descriptor activity before it next returns.

Solution: give run_toplevel_callbacks a return value which says
whether it's actually done something, and if so, return immediately in
case that was the droid the caller was looking for. As it were.
This commit is contained in:
Simon Tatham 2018-05-24 16:54:16 +01:00
parent 7d0ade7eac
commit b8c4d042bd
3 changed files with 15 additions and 4 deletions

View File

@ -97,8 +97,10 @@ void queue_toplevel_callback(toplevel_callback_fn_t fn, void *ctx)
cb->next = NULL;
}
void run_toplevel_callbacks(void)
int run_toplevel_callbacks(void)
{
int done_something = FALSE;
if (cbhead) {
/*
* Transfer the head callback into cbcurr to indicate that
@ -117,7 +119,10 @@ void run_toplevel_callbacks(void)
cbcurr->fn(cbcurr->ctx);
sfree(cbcurr);
cbcurr = NULL;
done_something = TRUE;
}
return done_something;
}
int toplevel_callback_pending(void)

View File

@ -1595,10 +1595,15 @@ unsigned long timing_last_clock(void);
* 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.
*
* run_toplevel_callbacks() returns TRUE if it ran any actual code.
* This can be used as a means of speculatively terminating a select
* loop, as in PSFTP, for example - if a callback has run then perhaps
* it might have done whatever the loop's caller was waiting for.
*/
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 run_toplevel_callbacks(void);
int toplevel_callback_pending(void);
void delete_callbacks_for_context(void *ctx);

View File

@ -459,6 +459,7 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok)
int fd, fdstate, rwx, ret, maxfd;
unsigned long now = GETTICKCOUNT();
unsigned long next;
int done_something = FALSE;
fdlist = NULL;
fdcount = fdsize = 0;
@ -509,7 +510,7 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok)
tv.tv_usec = 0;
ret = select(maxfd, &rset, &wset, &xset, &tv);
if (ret == 0)
run_toplevel_callbacks();
done_something |= run_toplevel_callbacks();
} else if (run_timers(now, &next)) {
do {
unsigned long then;
@ -535,7 +536,7 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok)
ret = select(maxfd, &rset, &wset, &xset, NULL);
} while (ret < 0 && errno == EINTR);
}
} while (ret == 0);
} while (ret == 0 && !done_something);
if (ret < 0) {
perror("select");