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

winsftp.c: avoid creating multiple netevents.

The do_select function is called with a boolean parameter indicating
whether we're supposed to start or stop paying attention to network
activity on a given socket. So if we freeze and unfreeze the socket in
mid-session because of backlog, we'll call do_select(s, false) to
freeze it, and do_select(s, true) to unfreeze it.

But the implementation of do_select in the Windows SFTP code predated
the rigorous handling of socket backlogs, so it assumed that
do_select(s, true) would only be called at initialisation time, i.e.
only once, and therefore that it was safe to use that flag as a cue to
set up the Windows event object to associate with socket activity.
Hence, every time the socket was frozen and unfrozen, we would create
a new netevent at unfreeze time, leaking the old one.

I think perhaps part of the reason why that was hard to figure out was
that the boolean parameter was called 'startup' rather than 'enable'.
To make it less confusing the next time I read this code, I've also
renamed it, and while I was at it, adjusted another related comment.

(cherry picked from commit bd5c957e5b)
This commit is contained in:
Simon Tatham 2019-12-21 13:31:02 +00:00
parent ef936e72a2
commit 3d44cef8ea
4 changed files with 14 additions and 10 deletions

View File

@ -1026,10 +1026,10 @@ void cleanup_exit(int code)
/* /*
* Set up, or shut down, an AsyncSelect. Called from winnet.c. * Set up, or shut down, an AsyncSelect. Called from winnet.c.
*/ */
char *do_select(SOCKET skt, bool startup) char *do_select(SOCKET skt, bool enable)
{ {
int msg, events; int msg, events;
if (startup) { if (enable) {
msg = WM_NETEVENT; msg = WM_NETEVENT;
events = (FD_CONNECT | FD_READ | FD_WRITE | events = (FD_CONNECT | FD_READ | FD_WRITE |
FD_OOB | FD_CLOSE | FD_ACCEPT); FD_OOB | FD_CLOSE | FD_ACCEPT);

View File

@ -190,10 +190,10 @@ static void version(void)
exit(0); exit(0);
} }
char *do_select(SOCKET skt, bool startup) char *do_select(SOCKET skt, bool enable)
{ {
int events; int events;
if (startup) { if (enable) {
events = (FD_CONNECT | FD_READ | FD_WRITE | events = (FD_CONNECT | FD_READ | FD_WRITE |
FD_OOB | FD_CLOSE | FD_ACCEPT); FD_OOB | FD_CLOSE | FD_ACCEPT);
} else { } else {

View File

@ -467,19 +467,21 @@ char *dir_file_cat(const char *dir, const char *file)
*/ */
static SOCKET sftp_ssh_socket = INVALID_SOCKET; static SOCKET sftp_ssh_socket = INVALID_SOCKET;
static HANDLE netevent = INVALID_HANDLE_VALUE; static HANDLE netevent = INVALID_HANDLE_VALUE;
char *do_select(SOCKET skt, bool startup) char *do_select(SOCKET skt, bool enable)
{ {
int events; int events;
if (startup) if (enable)
sftp_ssh_socket = skt; sftp_ssh_socket = skt;
else else
sftp_ssh_socket = INVALID_SOCKET; sftp_ssh_socket = INVALID_SOCKET;
if (netevent == INVALID_HANDLE_VALUE)
netevent = CreateEvent(NULL, false, false, NULL);
if (p_WSAEventSelect) { if (p_WSAEventSelect) {
if (startup) { if (enable) {
events = (FD_CONNECT | FD_READ | FD_WRITE | events = (FD_CONNECT | FD_READ | FD_WRITE |
FD_OOB | FD_CLOSE | FD_ACCEPT); FD_OOB | FD_CLOSE | FD_ACCEPT);
netevent = CreateEvent(NULL, false, false, NULL);
} else { } else {
events = 0; events = 0;
} }
@ -732,7 +734,9 @@ char *ssh_sftp_get_cmdline(const char *prompt, bool no_fds_ok)
do { do {
ret = do_eventsel_loop(ctx->event); ret = do_eventsel_loop(ctx->event);
/* Error return can only occur if netevent==NULL, and it ain't. */ /* do_eventsel_loop can't return an error (unlike
* ssh_sftp_loop_iteration, which can return -1 if select goes
* wrong or if the socket doesn't exist). */
assert(ret >= 0); assert(ret >= 0);
} while (ret == 0); } while (ret == 0);

View File

@ -350,7 +350,7 @@ DECL_WINDOWS_FUNCTION(GLOBAL, int, select,
* Provided by each client of winnet.c, and called by winnet.c to turn * Provided by each client of winnet.c, and called by winnet.c to turn
* on or off WSA*Select for a given socket. * on or off WSA*Select for a given socket.
*/ */
char *do_select(SOCKET skt, bool startup); char *do_select(SOCKET skt, bool enable);
/* /*
* Network-subsystem-related functions provided in other Windows modules. * Network-subsystem-related functions provided in other Windows modules.