From f0fbbe236452ae27a7577deafbbc44ace2c209e7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 25 Nov 2008 13:33:07 +0000 Subject: [PATCH] trying to resolve GSocketManager API mess: the meaning of Install/Uninstall_Callback() and Enable/Disable_Events() has diverged in different ports and didn't make any sense any more; merge them in a single function (with still differing semantics though); also added Close_Socket() git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@56964 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/gsocket.h | 31 +++++++++--- include/wx/msw/gsockmsw.h | 2 - include/wx/unix/gsockunx.h | 54 ++++++++++++++------- src/common/socket.cpp | 9 ++++ src/msw/gsocket.cpp | 15 ++---- src/msw/gsockmsw.cpp | 37 ++++++-------- src/osx/core/gsockosx.cpp | 99 +++++++++++++++++++++----------------- src/unix/gsocket.cpp | 76 ++++++++++++----------------- 8 files changed, 174 insertions(+), 149 deletions(-) diff --git a/include/wx/gsocket.h b/include/wx/gsocket.h index d674c3e361..c164f6477c 100644 --- a/include/wx/gsocket.h +++ b/include/wx/gsocket.h @@ -144,16 +144,33 @@ public: virtual void OnExit() = 0; - // do manager-specific socket initializations (and undo it): this is called - // in the beginning/end of the socket initialization/destruction + // do manager-specific socket initializations: called in the beginning of + // the socket initialization virtual bool Init_Socket(GSocket *socket) = 0; + + // called when the socket is being closed + // + // TODO: merge this with Destroy_Socket(), currently 2 separate functions + // are needed because Init_Socket() always allocates manager-specific + // resources in GSocket and Destroy_Socket() must be called even if + // the socket has never been opened, but if the allocation were done + // on demand, then Destroy_Socket() could be called from + // GSocket::Close() and we wouldn't need Close_Socket() at all + virtual void Close_Socket(GSocket *socket) = 0; + + // undo Init_Socket(): called from GSocket dtor virtual void Destroy_Socket(GSocket *socket) = 0; - virtual void Install_Callback(GSocket *socket, GSocketEvent event) = 0; - virtual void Uninstall_Callback(GSocket *socket, GSocketEvent event) = 0; - virtual void Enable_Events(GSocket *socket) = 0; - virtual void Disable_Events(GSocket *socket) = 0; + // these functions enable or disable monitoring of the given socket for the + // specified events inside the currently running event loop (but notice + // that both BSD and Winsock implementations actually use socket->m_server + // value to determine what exactly should be monitored so it needs to be + // set before calling these functions) + virtual void Install_Callback(GSocket *socket, + GSocketEvent event = GSOCK_MAX_EVENT) = 0; + virtual void Uninstall_Callback(GSocket *socket, + GSocketEvent event = GSOCK_MAX_EVENT) = 0; virtual ~GSocketManager() { } @@ -190,7 +207,7 @@ public: virtual GSocket *WaitConnection(wxSocketBase& wxsocket) = 0; - virtual void Close() = 0; + void Close(); virtual void Shutdown(); void SetInitialSocketBuffers(int recv, int send) diff --git a/include/wx/msw/gsockmsw.h b/include/wx/msw/gsockmsw.h index 6270be5678..3fbd6ac1aa 100644 --- a/include/wx/msw/gsockmsw.h +++ b/include/wx/msw/gsockmsw.h @@ -33,8 +33,6 @@ public: m_msgnumber = 0; } - virtual void Close(); - virtual GSocket *WaitConnection(wxSocketBase& wxsocket); diff --git a/include/wx/unix/gsockunx.h b/include/wx/unix/gsockunx.h index 0d57fe5a2f..d0ccb7b9ed 100644 --- a/include/wx/unix/gsockunx.h +++ b/include/wx/unix/gsockunx.h @@ -19,7 +19,6 @@ public: GSocket(wxSocketBase& wxsocket); virtual ~GSocket(); - virtual void Close(); virtual void Shutdown(); virtual GSocket *WaitConnection(wxSocketBase& wxsocket); @@ -41,12 +40,35 @@ public: void Detected_Read(); void Detected_Write(); -protected: - //enable or disable event callback using gsocket gui callback table - void EnableEvents(bool flag = true); - void DisableEvents() { EnableEvents(false); } - void Enable(GSocketEvent event); - void Disable(GSocketEvent event); +private: + // enable or disable notifications for socket input/output events but only + // if m_use_events is true; do nothing otherwise + void EnableEvents() + { + if ( m_use_events ) + DoEnableEvents(true); + } + + void DisableEvents() + { + if ( m_use_events ) + DoEnableEvents(false); + } + + // really enable or disable socket input/output events, regardless of + // m_use_events value + void DoEnableEvents(bool enable); + + + // enable or disable events for the given event if m_use_events; do nothing + // otherwise + // + // notice that these functions also update m_detected: EnableEvent() clears + // the corresponding bit in it and DisableEvent() sets it + void EnableEvent(GSocketEvent event); + void DisableEvent(GSocketEvent event); + + GSocketError Input_Timeout(); GSocketError Output_Timeout(); int Recv_Stream(char *buffer, int size); @@ -92,20 +114,18 @@ public: return true; } - virtual void Destroy_Socket(GSocket *socket) - { - free(socket->m_gui_dependent); - } - virtual void Enable_Events(GSocket *socket) - { - Install_Callback(socket, GSOCK_INPUT); - Install_Callback(socket, GSOCK_OUTPUT); - } - virtual void Disable_Events(GSocket *socket) + virtual void Close_Socket(GSocket *socket) { Uninstall_Callback(socket, GSOCK_INPUT); Uninstall_Callback(socket, GSOCK_OUTPUT); + + close(socket->m_fd); + } + + virtual void Destroy_Socket(GSocket *socket) + { + free(socket->m_gui_dependent); } protected: diff --git a/src/common/socket.cpp b/src/common/socket.cpp index a1e3be8456..2487fddcfb 100644 --- a/src/common/socket.cpp +++ b/src/common/socket.cpp @@ -210,6 +210,15 @@ GSocketBase::~GSocketBase() GSocketManager::Get()->Destroy_Socket(static_cast(this)); } +void GSocketBase::Close() +{ + if ( m_fd != INVALID_SOCKET ) + { + GSocketManager::Get()->Close_Socket(static_cast(this)); + m_fd = INVALID_SOCKET; + } +} + /* GSocket_Shutdown: * Disallow further read/write operations on this socket, close * the fd and disable all callbacks. diff --git a/src/msw/gsocket.cpp b/src/msw/gsocket.cpp index d701ed4caf..cb88ba7af2 100644 --- a/src/msw/gsocket.cpp +++ b/src/msw/gsocket.cpp @@ -101,13 +101,6 @@ void GSocket_Cleanup() /* Constructors / Destructors for GSocket */ -void GSocket::Close() -{ - GSocketManager::Get()->Disable_Events(this); - closesocket(m_fd); - m_fd = INVALID_SOCKET; -} - /* Server specific parts */ /* GSocket_SetServer: @@ -152,7 +145,7 @@ GSocketError GSocket::SetServer() } ioctlsocket(m_fd, FIONBIO, (u_long FAR *) &arg); - GSocketManager::Get()->Enable_Events(this); + GSocketManager::Get()->Install_Callback(this); /* allow a socket to re-bind if the socket is in the TIME_WAIT state after being previously closed. @@ -262,7 +255,7 @@ GSocket *GSocket::WaitConnection(wxSocketBase& wxsocket) } ioctlsocket(connection->m_fd, FIONBIO, (u_long FAR *) &arg); - GSocketManager::Get()->Enable_Events(connection); + GSocketManager::Get()->Install_Callback(connection); return connection; } @@ -374,7 +367,7 @@ GSocketError GSocket::Connect(GSocketStream stream) } ioctlsocket(m_fd, FIONBIO, (u_long FAR *) &arg); - GSocketManager::Get()->Enable_Events(this); + GSocketManager::Get()->Install_Callback(this); // If the reuse flag is set, use the applicable socket reuse flag if (m_reusable) @@ -485,7 +478,7 @@ GSocketError GSocket::SetNonOriented() } ioctlsocket(m_fd, FIONBIO, (u_long FAR *) &arg); - GSocketManager::Get()->Enable_Events(this); + GSocketManager::Get()->Install_Callback(this); if (m_reusable) { diff --git a/src/msw/gsockmsw.cpp b/src/msw/gsockmsw.cpp index 1975b983a0..f2cf192df8 100644 --- a/src/msw/gsockmsw.cpp +++ b/src/msw/gsockmsw.cpp @@ -185,13 +185,11 @@ public: virtual void OnExit(); virtual bool Init_Socket(GSocket *socket); + virtual void Close_Socket(GSocket *socket); virtual void Destroy_Socket(GSocket *socket); virtual void Install_Callback(GSocket *socket, GSocketEvent event); virtual void Uninstall_Callback(GSocket *socket, GSocketEvent event); - - virtual void Enable_Events(GSocket *socket); - virtual void Disable_Events(GSocket *socket); }; /* Global initializers */ @@ -309,6 +307,13 @@ bool GSocketMSWManager::Init_Socket(GSocket *socket) return true; } +void GSocketMSWManager::Close_Socket(GSocket *socket) +{ + Uninstall_Callback(socket, GSOCK_MAX_EVENT /* unused anyhow */); + + closesocket(socket->m_fd); +} + void GSocketMSWManager::Destroy_Socket(GSocket *socket) { /* Remove the socket from the list */ @@ -331,18 +336,6 @@ void GSocketMSWManager::Destroy_Socket(GSocket *socket) LeaveCriticalSection(&critical); } -void GSocketMSWManager::Install_Callback(GSocket * WXUNUSED(socket), - GSocketEvent WXUNUSED(event)) -{ - wxFAIL_MSG( _T("not used under MSW") ); -} - -void GSocketMSWManager::Uninstall_Callback(GSocket * WXUNUSED(socket), - GSocketEvent WXUNUSED(event)) -{ - wxFAIL_MSG( _T("not used under MSW") ); -} - /* Windows proc for asynchronous event handling */ LRESULT CALLBACK _GSocket_Internal_WinProc(HWND hWnd, @@ -405,12 +398,13 @@ LRESULT CALLBACK _GSocket_Internal_WinProc(HWND hWnd, return DefWindowProc(hWnd, uMsg, wParam, lParam); } -/* _GSocket_Enable_Events: +/* * Enable all event notifications; we need to be notified of all * events for internal processing, but we will only notify users - * when an appropiate callback function has been installed. + * when an appropriate callback function has been installed. */ -void GSocketMSWManager::Enable_Events(GSocket *socket) +void GSocketMSWManager::Install_Callback(GSocket *socket, + GSocketEvent WXUNUSED(event)) { if (socket->m_fd != INVALID_SOCKET) { @@ -438,10 +432,11 @@ void GSocketMSWManager::Enable_Events(GSocket *socket) } } -/* _GSocket_Disable_Events: - * Disable event notifications (when shutdowning the socket) +/* + * Disable event notifications (used when shutting down the socket) */ -void GSocketMSWManager::Disable_Events(GSocket *socket) +void GSocketMSWManager::Uninstall_Callback(GSocket *socket, + GSocketEvent WXUNUSED(event)) { if (socket->m_fd != INVALID_SOCKET) { diff --git a/src/osx/core/gsockosx.cpp b/src/osx/core/gsockosx.cpp index 5c5660fafa..b3b7114e9f 100644 --- a/src/osx/core/gsockosx.cpp +++ b/src/osx/core/gsockosx.cpp @@ -15,6 +15,17 @@ #include +namespace +{ + +// ---------------------------------------------------------------------------- +// global variables +// ---------------------------------------------------------------------------- + +// Sockets must use the event loop to monitor the events so we store a +// reference to the main thread event loop here +static CFRunLoopRef gs_mainRunLoop = NULL; + // ---------------------------------------------------------------------------- // Mac-specific data associated with each socket by GSocketCFManager // ---------------------------------------------------------------------------- @@ -61,16 +72,33 @@ public: m_source = CFSocketCreateRunLoopSource(NULL, m_socket, 0); - return m_source != NULL; + if ( !m_source ) + { + CFRelease(m_socket); + return false; + } + + CFRunLoopAddSource(gs_mainRunLoop, m_source, kCFRunLoopCommonModes); + + return true; + } + + // close the socket if it was opened + void Close() + { + // VZ: CFRunLoopRemoveSource() is probably unnecessary as + // CFSocketInvalidate() seems to do it internally from reading the + // docs, please remove it (and this comment) after testing + CFRunLoopRemoveSource(gs_mainRunLoop, m_source, kCFRunLoopCommonModes); + CFSocketInvalidate(m_socket); + + CFRelease(m_source); + CFRelease(m_socket); } - // free the objects created by Initialize() ~MacGSocketData() { - if ( m_source ) - CFRelease(m_source); - if ( m_socket ) - CFRelease(m_socket); + wxASSERT_MSG( !m_source && !m_socket, "forgot to call Close()?" ); } // return true if Initialize() had already been called successfully @@ -138,6 +166,9 @@ private: DECLARE_NO_COPY_CLASS(MacGSocketData); }; +} // anonymous namespace + + // ---------------------------------------------------------------------------- // CoreFoundation implementation of GSocketManager // ---------------------------------------------------------------------------- @@ -149,14 +180,12 @@ public: virtual void OnExit(); virtual bool Init_Socket(GSocket *socket); + virtual void Close_Socket(GSocket *socket); virtual void Destroy_Socket(GSocket *socket); virtual void Install_Callback(GSocket *socket, GSocketEvent event); virtual void Uninstall_Callback(GSocket *socket, GSocketEvent event); - virtual void Enable_Events(GSocket *socket); - virtual void Disable_Events(GSocket *socket); - private: // retrieve our custom data associated with the given socket // @@ -191,29 +220,22 @@ private: // differently depending on whether they happen on a server or on a client // socket) static int GetCFCallback(GSocket *socket, GSocketEvent event); - - - // Sockets must use the event loop on the main thread so we store a - // reference to the main loop here in OnInit() - static CFRunLoopRef ms_mainRunLoop; }; -CFRunLoopRef GSocketCFManager::ms_mainRunLoop = NULL; - bool GSocketCFManager::OnInit() { // No need to store the main loop again - if (ms_mainRunLoop != NULL) + if (gs_mainRunLoop != NULL) return true; // Get the loop for the main thread so our events will actually fire. // The common socket.cpp code will assert if initialize is called from a // secondary thread, otherwise Mac would have the same problems as MSW - ms_mainRunLoop = CFRunLoopGetCurrent(); - if ( !ms_mainRunLoop ) + gs_mainRunLoop = CFRunLoopGetCurrent(); + if ( !gs_mainRunLoop ) return false; - CFRetain(ms_mainRunLoop); + CFRetain(gs_mainRunLoop); return true; } @@ -221,8 +243,8 @@ bool GSocketCFManager::OnInit() void GSocketCFManager::OnExit() { // Release the reference count, and set the reference back to NULL - CFRelease(ms_mainRunLoop); - ms_mainRunLoop = NULL; + CFRelease(gs_mainRunLoop); + gs_mainRunLoop = NULL; } bool GSocketCFManager::Init_Socket(GSocket *socket) @@ -231,6 +253,16 @@ bool GSocketCFManager::Init_Socket(GSocket *socket) return true; } +void GSocketCFManager::Close_Socket(GSocket *socket) +{ + Uninstall_Callback(socket, GSOCK_INPUT); + Uninstall_Callback(socket, GSOCK_OUTPUT); + + MacGSocketData * const data = GetData(socket); + if ( data ) + data->Close(); +} + void GSocketCFManager::Destroy_Socket(GSocket *socket) { MacGSocketData * const data = GetData(socket); @@ -285,29 +317,6 @@ void GSocketCFManager::Uninstall_Callback(GSocket *socket, GSocketEvent event) CFSocketDisableCallBacks(data->GetSocket(), GetCFCallback(socket, event)); } -void GSocketCFManager::Enable_Events(GSocket *socket) -{ - const MacGSocketData * const data = GetInitializedData(socket); - if ( !data ) - return; - - CFRunLoopAddSource(ms_mainRunLoop, data->GetSource(), kCFRunLoopCommonModes); -} - -void GSocketCFManager::Disable_Events(GSocket *socket) -{ - const MacGSocketData * const data = GetInitializedData(socket); - if ( !data ) - return; - - // CFSocketInvalidate does CFRunLoopRemoveSource anyway - CFRunLoopRemoveSource(ms_mainRunLoop, data->GetSource(), kCFRunLoopCommonModes); - CFSocketInvalidate(data->GetSocket()); - - // CFSocketInvalidate has closed the socket so we want to make sure GSocket knows this - socket->m_fd = -1; -} - GSocketManager *wxAppTraits::GetSocketManager() { static GSocketCFManager s_manager; diff --git a/src/unix/gsocket.cpp b/src/unix/gsocket.cpp index 95d59e581b..6efa6b76ad 100644 --- a/src/unix/gsocket.cpp +++ b/src/unix/gsocket.cpp @@ -453,7 +453,7 @@ void GSocket_Cleanup() /* Constructors / Destructors for GSocket */ GSocket::GSocket(wxSocketBase& wxsocket) - : GSocketBase(wxsocket) + : GSocketBase(wxsocket) { m_handler = NULL; @@ -461,24 +461,6 @@ GSocket::GSocket(wxSocketBase& wxsocket) m_use_events = false; } -void GSocket::Close() -{ - if (m_use_events) - DisableEvents(); - - /* When running on OS X, the gsockosx implementation of GSocketGUIFunctionsTable - will close the socket during Disable_Events. However, it will only do this - if it is being used. That is, it won't do it in a console program. To - ensure we get the right behavior, we have gsockosx set m_fd = INVALID_SOCKET - if it has closed the socket which indicates to us (at runtime, instead of - at compile time as this had been before) that the socket has already - been closed. - */ - if(m_fd != INVALID_SOCKET) - close(m_fd); - m_fd = INVALID_SOCKET; -} - GSocket::~GSocket() { delete m_handler; @@ -491,8 +473,7 @@ GSocket::~GSocket() void GSocket::Shutdown() { /* Don't allow events to fire after socket has been closed */ - if (m_use_events) - DisableEvents(); + DisableEvents(); GSocketBase::Shutdown(); } @@ -548,8 +529,7 @@ GSocketError GSocket::SetServer() #endif ioctl(m_fd, FIONBIO, &arg); - if (m_use_events) - EnableEvents(); + EnableEvents(); /* allow a socket to re-bind if the socket is in the TIME_WAIT state after being previously closed. @@ -629,7 +609,7 @@ GSocket *GSocket::WaitConnection(wxSocketBase& wxsocket) connection->m_fd = accept(m_fd, (sockaddr*)&from, (WX_SOCKLEN_T *) &fromlen); /* Reenable CONNECTION events */ - Enable(GSOCK_CONNECTION); + EnableEvent(GSOCK_CONNECTION); if (connection->m_fd == INVALID_SOCKET) { @@ -679,15 +659,22 @@ void GSocket::Notify(bool flag) if (flag == m_use_events) return; m_use_events = flag; - EnableEvents(flag); + DoEnableEvents(flag); } -void GSocket::EnableEvents(bool flag) +void GSocket::DoEnableEvents(bool flag) { - if (flag) - GSocketManager::Get()->Enable_Events(this); - else - GSocketManager::Get()->Disable_Events(this); + GSocketManager * const manager = GSocketManager::Get(); + if ( flag ) + { + manager->Install_Callback(this, GSOCK_INPUT); + manager->Install_Callback(this, GSOCK_OUTPUT); + } + else // off + { + manager->Uninstall_Callback(this, GSOCK_INPUT); + manager->Uninstall_Callback(this, GSOCK_OUTPUT); + } } bool GSocket::SetReusable() @@ -756,7 +743,7 @@ GSocketError GSocket::Connect(GSocketStream stream) assert(this); /* Enable CONNECTION events (needed for nonblocking connections) */ - Enable(GSOCK_CONNECTION); + EnableEvent(GSOCK_CONNECTION); if (m_fd != INVALID_SOCKET) { @@ -819,15 +806,14 @@ GSocketError GSocket::Connect(GSocketStream stream) /* Connect it to the peer address, with a timeout (see below) */ ret = connect(m_fd, m_peer->m_addr, m_peer->m_len); - /* We only call Enable_Events if we know we aren't shutting down the socket. - * NB: Enable_Events needs to be called whether the socket is blocking or + /* We only call EnableEvents() if we know we aren't shutting down the socket. + * NB: EnableEvents() needs to be called whether the socket is blocking or * non-blocking, it just shouldn't be called prior to knowing there is a * connection _if_ blocking sockets are being used. * If connect above returns 0, we are already connected and need to make the - * call to Enable_Events now. + * call to EnableEvents() now. */ - - if (m_use_events && (m_non_blocking || ret == 0)) + if ( m_non_blocking || (ret == 0) ) EnableEvents(); if (ret == -1) @@ -853,8 +839,7 @@ GSocketError GSocket::Connect(GSocketStream stream) SOCKOPTLEN_T len = sizeof(error); getsockopt(m_fd, SOL_SOCKET, SO_ERROR, (char*) &error, &len); - if (m_use_events) - EnableEvents(); + EnableEvents(); if (!error) return GSOCK_NOERROR; @@ -934,8 +919,7 @@ GSocketError GSocket::SetNonOriented() #else ioctl(m_fd, FIONBIO, &arg); #endif - if (m_use_events) - EnableEvents(); + EnableEvents(); if (m_reusable) { @@ -983,7 +967,7 @@ int GSocket::Read(char *buffer, int size) } /* Disable events during query of socket status */ - Disable(GSOCK_INPUT); + DisableEvent(GSOCK_INPUT); /* If the socket is blocking, wait for data (with a timeout) */ if (Input_Timeout() == GSOCK_TIMEDOUT) { @@ -1029,7 +1013,7 @@ int GSocket::Read(char *buffer, int size) } /* Enable events again now that we are done processing */ - Enable(GSOCK_INPUT); + EnableEvent(GSOCK_INPUT); return ret; } @@ -1082,7 +1066,7 @@ int GSocket::Write(const char *buffer, int size) * that the socket is writable until a read operation fails. Only then * will further OUTPUT events be posted. */ - Enable(GSOCK_OUTPUT); + EnableEvent(GSOCK_OUTPUT); return -1; } @@ -1137,7 +1121,7 @@ GSocketError GSocket::SetSockOpt(int level, int optname, return GSOCK_OPTERR; } -void GSocket::Enable(GSocketEvent event) +void GSocket::EnableEvent(GSocketEvent event) { if (m_use_events) { @@ -1146,7 +1130,7 @@ void GSocket::Enable(GSocketEvent event) } } -void GSocket::Disable(GSocketEvent event) +void GSocket::DisableEvent(GSocketEvent event) { if (m_use_events) { @@ -1355,7 +1339,7 @@ int GSocket::Send_Dgram(const char *buffer, int size) void GSocket::OnStateChange(GSocketEvent event) { - Disable(event); + DisableEvent(event); NotifyOnStateChange(event); if ( event == GSOCK_LOST ) -- 2.45.2