From ef224dbe4152417629fec31b9908a7d1f0f721f4 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Fri, 1 Nov 2019 15:35:41 +0100
Subject: [PATCH 1/2] Add unit test for reading from wxSocket in a thread

Check that reading from blocking socket in a thread works.
---
 tests/net/socket.cpp | 60 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/tests/net/socket.cpp b/tests/net/socket.cpp
index ca21bc9971..699634e95a 100644
--- a/tests/net/socket.cpp
+++ b/tests/net/socket.cpp
@@ -40,6 +40,14 @@ class SocketTestCase : public CppUnit::TestCase
 public:
     SocketTestCase() { }
 
+    // get the address to connect to, if NULL is returned it means that the
+    // test is disabled and shouldn't run at all
+    static wxSockAddress* GetServer();
+
+    // get the socket to read HTTP reply from, returns NULL if the test is
+    // disabled
+    static wxSocketClient* GetHTTPSocket(int flags = wxSOCKET_NONE);
+
 private:
     // we need to repeat the tests twice as the sockets behave differently when
     // there is an active event loop and without it
@@ -50,6 +58,7 @@ private:
         CPPUNIT_TEST( ReadBlock ); \
         CPPUNIT_TEST( ReadNowait ); \
         CPPUNIT_TEST( ReadWaitall ); \
+        CPPUNIT_TEST( ReadAnotherThread ); \
         CPPUNIT_TEST( UrlTest )
 
     CPPUNIT_TEST_SUITE( SocketTestCase );
@@ -86,14 +95,6 @@ private:
         wxEventLoopBase *m_evtLoopOld;
     };
 
-    // get the address to connect to, if NULL is returned it means that the
-    // test is disabled and shouldn't run at all
-    wxSockAddress* GetServer() const;
-
-    // get the socket to read HTTP reply from, returns NULL if the test is
-    // disabled
-    wxSocketClient* GetHTTPSocket(int flags = wxSOCKET_NONE) const;
-
     void PseudoTest_SetUseEventLoop() { ms_useLoop = true; }
 
     void BlockingConnect();
@@ -102,6 +103,7 @@ private:
     void ReadBlock();
     void ReadNowait();
     void ReadWaitall();
+    void ReadAnotherThread();
 
     void UrlTest();
 
@@ -115,7 +117,7 @@ bool SocketTestCase::ms_useLoop = false;
 CPPUNIT_TEST_SUITE_REGISTRATION( SocketTestCase );
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( SocketTestCase, "SocketTestCase" );
 
-wxSockAddress* SocketTestCase::GetServer() const
+wxSockAddress* SocketTestCase::GetServer()
 {
     if ( gs_serverHost.empty() )
         return NULL;
@@ -127,7 +129,7 @@ wxSockAddress* SocketTestCase::GetServer() const
     return addr;
 }
 
-wxSocketClient* SocketTestCase::GetHTTPSocket(int flags) const
+wxSocketClient* SocketTestCase::GetHTTPSocket(int flags)
 {
     wxSockAddress *addr = GetServer();
     if ( !addr )
@@ -248,6 +250,44 @@ void SocketTestCase::ReadWaitall()
     CPPUNIT_ASSERT_EQUAL( WXSIZEOF(buf), (size_t)sock->LastReadCount() );
 }
 
+void SocketTestCase::ReadAnotherThread()
+{
+    class SocketThread : public wxThread
+    {
+    public:
+        SocketThread()
+            : wxThread(wxTHREAD_JOINABLE)
+        {
+        }
+
+        virtual void* Entry() wxOVERRIDE
+        {
+            wxSocketClientPtr sock(SocketTestCase::GetHTTPSocket(wxSOCKET_BLOCK));
+            if ( !sock )
+                return NULL;
+
+            char bufSmall[128];
+            sock->Read(bufSmall, WXSIZEOF(bufSmall));
+
+            REQUIRE( sock->LastError() == wxSOCKET_NOERROR );
+            CHECK( sock->LastCount() == WXSIZEOF(bufSmall) );
+            CHECK( sock->LastReadCount() == WXSIZEOF(bufSmall) );
+
+            REQUIRE_NOTHROW( sock.reset() );
+
+            return NULL;
+        }
+    };
+
+    SocketThread thr;
+
+    SocketTestEventLoop loop(ms_useLoop);
+
+    thr.Run();
+
+    CHECK( thr.Wait() == NULL );
+}
+
 void SocketTestCase::UrlTest()
 {
     if ( gs_serverHost.empty() )

From 6bc30968c596423865a80af7d14301b80aaa954f Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Fri, 1 Nov 2019 16:00:22 +0100
Subject: [PATCH 2/2] Don't create CFSocket unnecessarily when closing wxSocket

DoClose() shouldn't do anything if CFSocket hadn't been created at all
(as will be the case for blocking sockets) instead of creating it only
to destroy it immediately afterwards.
---
 src/osx/core/sockosx.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/osx/core/sockosx.cpp b/src/osx/core/sockosx.cpp
index 899dce6234..f6301c28ca 100644
--- a/src/osx/core/sockosx.cpp
+++ b/src/osx/core/sockosx.cpp
@@ -63,6 +63,12 @@ public:
 private:
     virtual void DoClose() wxOVERRIDE
     {
+        // No need to do anything if we had never created the underlying
+        // socket: this avoids creating it from Uninstall_Callback() completely
+        // unnecessarily.
+        if ( !m_socket )
+            return;
+
         wxSocketManager * const manager = wxSocketManager::Get();
         if ( manager )
         {