Thread safe replacement of handles_by_evtomain global

Before it was used for local proxy command only, so probability of problems was relatively small. In PuTTY 0.75 it's used even for Pageant communication.

Source commit: f00fe4d8c44d99a0e7a80d9ebe0225d1ad297c22

source/core/SecureShell.cpp

@@ -77,10 +77,13 @@ __fastcall TSecureShell::TSecureShell(TSessionUI* UI,
   FWaitingForData = 0;
   FCallbackSet.reset(new callback_set());
   memset(FCallbackSet.get(), 0, sizeof(callback_set));
+  FCallbackSet->handles_by_evtomain = new_handles_by_evtomain();
 }
 //---------------------------------------------------------------------------
 __fastcall TSecureShell::~TSecureShell()
 {
+  freetree234(FCallbackSet->handles_by_evtomain);
+  FCallbackSet->handles_by_evtomain = NULL;
   DebugAssert(FWaiting == 0);
   Active = false;
   ResetConnection();
@@ -2003,10 +2006,8 @@ bool __fastcall TSecureShell::EventSelectLoop(unsigned int MSec, bool ReadEventR
         {
           sfree(Handles);
         }
-        // Note that this returns all handles, not only the this-session-related handles,
-        // so we can possibly be processing handles of other sessions, what may be very wrong.
         // It returns only busy handles, so the set can change with every call to run_toplevel_callbacks.
-        Handles = handle_get_events(&HandleCount);
+        Handles = handle_get_events(FCallbackSet->handles_by_evtomain, &HandleCount);
         Handles = sresize(Handles, HandleCount + 1, HANDLE);
         Handles[HandleCount] = FSocketEvent;
         WaitResult = WaitForMultipleObjects(HandleCount + 1, Handles, FALSE, TimeoutStep);
@@ -2027,7 +2028,7 @@ bool __fastcall TSecureShell::EventSelectLoop(unsigned int MSec, bool ReadEventR
 
       if (WaitResult < WAIT_OBJECT_0 + HandleCount)
       {
-        if (handle_got_event(Handles[WaitResult - WAIT_OBJECT_0]))
+        if (handle_got_event(FCallbackSet->handles_by_evtomain, Handles[WaitResult - WAIT_OBJECT_0]))
         {
           Result = true;
         }

source/putty/agentf.c

@@ -12,6 +12,7 @@
 #include "sshchan.h"
 
 typedef struct agentf {
+    Plug *plug; // WINSCP
     SshChannel *c;
     bufchain inbuffer;
     agent_pending_query *pending;
@@ -97,7 +98,7 @@ static void agentf_try_forward(agentf *af)
         bufchain_fetch_consume(
             &af->inbuffer, strbuf_append(message, length), length);
         af->pending = agent_query(
-            message, &reply, &replylen, agentf_callback, af);
+            message, &reply, &replylen, agentf_callback, af, get_callback_set(af->plug)); // WINSCP
         strbuf_free(message);
 
         if (af->pending)
@@ -173,9 +174,10 @@ static const ChannelVtable agentf_channelvt = {
     /*.request_response =*/ chan_no_request_response,
 };
 
-Channel *agentf_new(SshChannel *c)
+Channel *agentf_new(SshChannel *c, Plug *plug) // WINSCP
 {
     agentf *af = snew(agentf);
+    af->plug = plug; // WINSCP
     af->c = c;
     af->chan.vt = &agentf_channelvt;
     af->chan.initial_fixed_window_size = 0;

source/putty/putty.h

@@ -2072,7 +2072,7 @@ int mk_wcswidth_cjk(const unsigned int *pwcs, size_t n);
 typedef struct agent_pending_query agent_pending_query;
 agent_pending_query *agent_query(
     strbuf *in, void **out, int *outlen,
-    void (*callback)(void *, void *, int), void *callback_ctx);
+    void (*callback)(void *, void *, int), void *callback_ctx, struct callback_set * callback_set); // WINSCP
 void agent_cancel_query(agent_pending_query *);
 void agent_query_synchronous(strbuf *in, void **out, int *outlen);
 bool agent_exists(void);
@@ -2376,6 +2376,7 @@ struct callback_set {
     struct callback *cbcurr, *cbhead, *cbtail;
     IdempotentCallback * ic_pktin_free;
     PacketQueueNode * pktin_freeq_head;
+    tree234 * handles_by_evtomain;
 };
 #define CALLBACK_SET_ONLY struct callback_set * callback_set_v
 #define CALLBACK_SET CALLBACK_SET_ONLY,
@@ -2393,6 +2394,7 @@ void delete_callbacks_for_context(CALLBACK_SET void *ctx);
 LogPolicy *log_get_logpolicy(LogContext *ctx); // WINSCP
 Seat * get_log_seat(LogContext * lp); // WINSCP
 struct callback_set * get_log_callback_set(LogContext * lp); // WINSCP
+tree234 *new_handles_by_evtomain(); // WINSCP
 
 /*
  * Another facility in callback.c deals with 'idempotent' callbacks,

source/putty/ssh.c

@@ -270,7 +270,8 @@ static void ssh_got_ssh_version(struct ssh_version_receiver *rcv,
                     NULL
 #endif
                     ,conf_get_str(ssh->conf, CONF_loghost),
-                    conf_get_bool(ssh->conf, CONF_change_password) // WINSCP
+                    conf_get_bool(ssh->conf, CONF_change_password), // WINSCP
+                    ssh->seat
                     );
                 ssh_connect_ppl(ssh, userauth_layer);
                 transport_child_layer = userauth_layer;

source/putty/ssh.h

@@ -1226,7 +1226,7 @@ void x11_format_auth_for_authfile(
 int x11_identify_auth_proto(ptrlen protoname);
 void *x11_dehexify(ptrlen hex, int *outlen);
 
-Channel *agentf_new(SshChannel *c);
+Channel *agentf_new(SshChannel *c, Plug *plug); // WINSCP
 
 bool dh_is_gex(const ssh_kex *kex);
 dh_ctx *dh_setup_group(const ssh_kex *kex);

source/putty/ssh2connection-client.c

@@ -114,7 +114,7 @@ static ChanopenResult chan_open_auth_agent(
          * forwarded data stream ourselves for message boundaries, and
          * passing each individual message to the one-off agent_query().
          */
-        CHANOPEN_RETURN_SUCCESS(agentf_new(sc));
+        CHANOPEN_RETURN_SUCCESS(agentf_new(sc, plug));
     }
     } // WINSCP
 }

source/putty/ssh2userauth.c

@@ -95,6 +95,8 @@ struct ssh2_userauth_state {
     bool ki_scc_initialised;
     bool ki_printed_header;
 
+    Seat *seat; // WINSCP
+
     PacketProtocolLayer ppl;
 };
 
@@ -142,10 +144,11 @@ PacketProtocolLayer *ssh2_userauth_new(
     const char *default_username, bool change_username,
     bool try_ki_auth, bool try_gssapi_auth, bool try_gssapi_kex_auth,
     bool gssapi_fwd, struct ssh_connection_shared_gss_state *shgss,
-    const char * loghost, bool change_password) // WINSCP
+    const char * loghost, bool change_password, Seat *seat) // WINSCP
 {
     struct ssh2_userauth_state *s = snew(struct ssh2_userauth_state);
     memset(s, 0, sizeof(*s));
+    s->seat = seat;
     s->ppl.vt = &ssh2_userauth_vtable;
 
     s->successor_layer = successor_layer;
@@ -1829,7 +1832,7 @@ static void ssh2_userauth_agent_query(
     s->agent_response_to_free = NULL;
 
     s->auth_agent_query = agent_query(req, &response, &response_len,
-                                      ssh2_userauth_agent_callback, s);
+                                      ssh2_userauth_agent_callback, s, get_seat_callback_set(s->seat)); // WINSCP
     if (!s->auth_agent_query)
         ssh2_userauth_agent_callback(s, response, response_len);
 }

source/putty/sshppl.h

@@ -125,7 +125,7 @@ PacketProtocolLayer *ssh2_userauth_new(
     bool try_ki_auth,
     bool try_gssapi_auth, bool try_gssapi_kex_auth,
     bool gssapi_fwd, struct ssh_connection_shared_gss_state *shgss,
-    const char * loghost, bool change_password); // WINSCP
+    const char * loghost, bool change_password, Seat *seat); // WINSCP
 PacketProtocolLayer *ssh2_connection_new(
     Ssh *ssh, ssh_sharing_state *connshare, bool is_simple,
     Conf *conf, const char *peer_verstring, ConnectionLayer **cl_out);

source/putty/windows/winhandl.c

@@ -402,7 +402,9 @@ struct handle {
     } u;
 };
 
+#ifndef WINSCP
 static tree234 *handles_by_evtomain;
+#endif
 
 static int handle_cmp_evtomain(void *av, void *bv)
 {
@@ -430,7 +432,7 @@ static int handle_find_evtomain(void *av, void *bv)
         return 0;
 }
 
-struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
+struct handle *handle_input_new(tree234 * handles_by_evtomain, HANDLE handle, handle_inputfn_t gotdata,
                                 void *privdata, int flags)
 {
     struct handle *h = snew(struct handle);
@@ -447,8 +449,10 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
     h->u.i.privdata = privdata;
     h->u.i.flags = flags;
 
+    #ifndef WINSCP
     if (!handles_by_evtomain)
         handles_by_evtomain = newtree234(handle_cmp_evtomain);
+    #endif
     add234(handles_by_evtomain, h);
 
     CreateThread(NULL, 0, handle_input_threadfunc,
@@ -458,7 +462,7 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
     return h;
 }
 
-struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
+struct handle *handle_output_new(tree234 * handles_by_evtomain, HANDLE handle, handle_outputfn_t sentdata, // WINSCP
                                  void *privdata, int flags)
 {
     struct handle *h = snew(struct handle);
@@ -478,8 +482,10 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
     h->u.o.sentdata = sentdata;
     h->u.o.flags = flags;
 
+    #ifndef WINSCP
     if (!handles_by_evtomain)
         handles_by_evtomain = newtree234(handle_cmp_evtomain);
+    #endif
     add234(handles_by_evtomain, h);
 
     CreateThread(NULL, 0, handle_output_threadfunc,
@@ -488,6 +494,7 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
     return h;
 }
 
+#ifndef WINSCP
 struct handle *handle_add_foreign_event(HANDLE event,
                                         void (*callback)(void *), void *ctx)
 {
@@ -511,6 +518,7 @@ struct handle *handle_add_foreign_event(HANDLE event,
 
     return h;
 }
+#endif
 
 size_t handle_write(struct handle *h, const void *data, size_t len)
 {
@@ -537,7 +545,7 @@ void handle_write_eof(struct handle *h)
     }
 }
 
-HANDLE *handle_get_events(int *nevents)
+HANDLE *handle_get_events(tree234 * handles_by_evtomain, int *nevents) // WINSCP
 {
     HANDLE *ret;
     struct handle *h;
@@ -563,7 +571,7 @@ HANDLE *handle_get_events(int *nevents)
     return ret;
 }
 
-static void handle_destroy(struct handle *h)
+static void handle_destroy(tree234 * handles_by_evtomain, struct handle *h) // WINSCP
 {
     if (h->type == HT_OUTPUT)
         bufchain_clear(&h->u.o.queued_data);
@@ -571,15 +579,9 @@ static void handle_destroy(struct handle *h)
     CloseHandle(h->u.g.ev_to_main);
     del234(handles_by_evtomain, h);
     sfree(h);
-    // WINSCP (memory leak)
-    if (count234(handles_by_evtomain) == 0)
-    {
-        freetree234(handles_by_evtomain);
-        handles_by_evtomain = NULL;
-    }
 }
 
-void handle_free(struct handle *h)
+void handle_free(tree234 * handles_by_evtomain, struct handle *h) // WINSCP
 {
     assert(h && !h->u.g.moribund);
     if (h->u.g.busy && h->type != HT_FOREIGN) {
@@ -600,7 +602,7 @@ void handle_free(struct handle *h)
          * There isn't even a subthread; we can go straight to
          * handle_destroy.
          */
-        handle_destroy(h);
+        handle_destroy(handles_by_evtomain, h); // WINSCP
     } else {
         /*
          * The subthread is alive but not busy, so we now signal it
@@ -614,7 +616,7 @@ void handle_free(struct handle *h)
     }
 }
 
-int handle_got_event(HANDLE event) // WINSCP
+int handle_got_event(tree234 * handles_by_evtomain, HANDLE event) // WINSCP
 {
     struct handle *h;
 
@@ -642,7 +644,7 @@ int handle_got_event(HANDLE event) // WINSCP
          * we haven't yet done so, or destroy the handle if not.
          */
         if (h->u.g.done) {
-            handle_destroy(h);
+            handle_destroy(handles_by_evtomain, h); // WINSCP
         } else {
             h->u.g.done = true;
             h->u.g.busy = true;
@@ -731,3 +733,9 @@ void handle_sink_init(handle_sink *sink, struct handle *h)
     sink->h = h;
     BinarySink_INIT(sink, handle_sink_write);
 }
+
+// WINSCP
+tree234 *new_handles_by_evtomain()
+{
+    return newtree234(handle_cmp_evtomain);
+}

source/putty/windows/winhsock.c

@@ -119,6 +119,7 @@ static Plug *sk_handle_plug(Socket *s, Plug *p)
 static void sk_handle_close(Socket *s)
 {
     HandleSocket *hs = container_of(s, HandleSocket, sock);
+    tree234 * handles_by_evtomain = get_callback_set(hs->plug)->handles_by_evtomain; // WINSCP
 
     if (hs->defer_close) {
         hs->deferred_close = true;
@@ -130,8 +131,8 @@ static void sk_handle_close(Socket *s)
     do_select(hs->plug, INVALID_SOCKET, 0);
     #endif
 
-    handle_free(hs->send_h);
-    handle_free(hs->recv_h);
+    handle_free(handles_by_evtomain, hs->send_h); // WINSCP
+    handle_free(handles_by_evtomain, hs->recv_h); // WINSCP
     CloseHandle(hs->send_H);
     if (hs->recv_H != hs->send_H)
         CloseHandle(hs->recv_H);
@@ -139,7 +140,7 @@ static void sk_handle_close(Socket *s)
 #ifdef MPEXT
     if (hs->stderr_h)
     {
-        handle_free(hs->stderr_h);
+        handle_free(handles_by_evtomain, hs->stderr_h); // WINSCP
     }
     if (hs->stderr_H)
     {
@@ -341,6 +342,7 @@ Socket *make_handle_socket(HANDLE send_H, HANDLE recv_H, HANDLE stderr_H,
 {
     HandleSocket *hs;
     int flags = (overlapped ? HANDLE_FLAG_OVERLAPPED : 0);
+    tree234 * handles_by_evtomain = get_callback_set(hs->plug)->handles_by_evtomain; // WINSCP
 
     hs = snew(HandleSocket);
     hs->sock.vt = &HandleSocket_sockvt;
@@ -351,12 +353,12 @@ Socket *make_handle_socket(HANDLE send_H, HANDLE recv_H, HANDLE stderr_H,
     psb_init(&hs->psb);
 
     hs->recv_H = recv_H;
-    hs->recv_h = handle_input_new(hs->recv_H, handle_gotdata, hs, flags);
+    hs->recv_h = handle_input_new(handles_by_evtomain, hs->recv_H, handle_gotdata, hs, flags); // WINSCP
     hs->send_H = send_H;
-    hs->send_h = handle_output_new(hs->send_H, handle_sentdata, hs, flags);
+    hs->send_h = handle_output_new(handles_by_evtomain, hs->send_H, handle_sentdata, hs, flags); // WINSCP
     hs->stderr_H = stderr_H;
     if (hs->stderr_H)
-        hs->stderr_h = handle_input_new(hs->stderr_H, handle_stderr,
+        hs->stderr_h = handle_input_new(handles_by_evtomain, hs->stderr_H, handle_stderr, // WINSCP
                                         hs, flags);
 
     hs->defer_close = hs->deferred_close = false;

source/putty/windows/winpgntc.c

@@ -172,6 +172,7 @@ struct agent_pending_query {
     strbuf *response;
     void (*callback)(void *, void *, int);
     void *callback_ctx;
+    struct callback_set *callback_set; // WINSCP
 };
 
 static int named_pipe_agent_accumulate_response(
@@ -221,7 +222,7 @@ static size_t named_pipe_agent_gotdata(
 
 static agent_pending_query *named_pipe_agent_query(
     strbuf *query, void **out, int *outlen,
-    void (*callback)(void *, void *, int), void *callback_ctx)
+    void (*callback)(void *, void *, int), void *callback_ctx, struct callback_set * callback_set) // WINSCP
 {
     agent_pending_query *pq = NULL;
     char *err = NULL, *pipename = NULL;
@@ -271,7 +272,8 @@ static agent_pending_query *named_pipe_agent_query(
     }
 
     pq = snew(agent_pending_query);
-    pq->handle = handle_input_new(pipehandle, named_pipe_agent_gotdata, pq, 0);
+    pq->callback_set = callback_set; // WINSCP
+    pq->handle = handle_input_new(callback_set->handles_by_evtomain, pipehandle, named_pipe_agent_gotdata, pq, 0); // WINSCP
     pipehandle = NULL;  /* prevent it being closed below */
     pq->response = strbuf_new_nm();
     pq->callback = callback;
@@ -296,7 +298,7 @@ static agent_pending_query *named_pipe_agent_query(
 
 void agent_cancel_query(agent_pending_query *pq)
 {
-    handle_free(pq->handle);
+    handle_free(pq->callback_set->handles_by_evtomain, pq->handle);
     if (pq->response)
         strbuf_free(pq->response);
     sfree(pq);
@@ -304,10 +306,10 @@ void agent_cancel_query(agent_pending_query *pq)
 
 agent_pending_query *agent_query(
     strbuf *query, void **out, int *outlen,
-    void (*callback)(void *, void *, int), void *callback_ctx)
+    void (*callback)(void *, void *, int), void *callback_ctx, struct callback_set * callback_set) // WINSCP
 {
     agent_pending_query *pq = named_pipe_agent_query(
-        query, out, outlen, callback, callback_ctx);
+        query, out, outlen, callback, callback_ctx, callback_set); // WINSCP
     if (pq || *out)
         return pq;
 

source/putty/windows/winstuff.h

@@ -618,19 +618,15 @@ 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 *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
+struct handle *handle_input_new(tree234 * handles_by_evtomain, HANDLE handle, handle_inputfn_t gotdata, // WINSCP
                                 void *privdata, int flags);
-struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
+struct handle *handle_output_new(tree234 * handles_by_evtomain, HANDLE handle, handle_outputfn_t sentdata, // WINSCP
                                  void *privdata, int flags);
 size_t handle_write(struct handle *h, const void *data, size_t len);
 void handle_write_eof(struct handle *h);
-HANDLE *handle_get_events(int *nevents);
-void handle_free(struct handle *h);
-#ifdef MPEXT
-int handle_got_event(HANDLE event);
-#else
-void handle_got_event(HANDLE event);
-#endif
+HANDLE *handle_get_events(tree234 * handles_by_evtomain, int *nevents); // WINSCP
+void handle_free(tree234 * handles_by_evtomain, struct handle *h); // WINSCP
+int handle_got_event(tree234 * handles_by_evtomain, HANDLE event); // WINSCP
 void handle_unthrottle(struct handle *h, size_t backlog);
 size_t handle_backlog(struct handle *h);
 void *handle_get_privdata(struct handle *h);