1 files changed, 45 insertions, 37 deletions

diff --git a/src/OSSupport/SocketThreads.h b/src/OSSupport/SocketThreads.h
index 858729c49..9e1947ab6 100644
--- a/src/OSSupport/SocketThreads.h
+++ b/src/OSSupport/SocketThreads.h
@@ -7,19 +7,20 @@
 
 /*
 Additional details:
-When a client is terminating a connection:
-- they call the StopReading() method to disable callbacks for the incoming data
-- they call the Write() method to queue any outstanding outgoing data
-- they call the QueueClose() method to queue the socket to close after outgoing data has been sent.
-When a socket slot is marked as having no callback, it is kept alive until its outgoing data queue is empty and its m_ShouldClose flag is set.
-This means that the socket can be written to several times before finally closing it via QueueClose()
+When a client wants to terminate the connection, they call the RemoveClient() function. This calls the
+callback one last time to read all the available outgoing data, putting it in the slot's m_OutgoingData
+buffer. Then it marks the slot as having no callback. The socket is kept alive until its outgoing data
+queue is empty, then shutdown is called on it and finally the socket is closed after a timeout.
+If at any time within this the remote end closes the socket, then the socket is closed directly.
+As soon as the socket is closed, the slot is finally removed from the SocketThread.
+The graph in $/docs/SocketThreads States.gv shows the state-machine transitions of the slot.
 */
 
 
 
 
 
-/// How many clients should one thread handle? (must be less than FD_SETSIZE for your platform)
+/** How many clients should one thread handle? (must be less than FD_SETSIZE for your platform) */
 #define MAX_SLOTS 63
 
 
@@ -27,8 +28,6 @@ This means that the socket can be written to several times before finally closin
 
 
 #pragma once
-#ifndef CSOCKETTHREADS_H_INCLUDED
-#define CSOCKETTHREADS_H_INCLUDED
 
 #include "Socket.h"
 #include "IsThread.h"
@@ -64,13 +63,13 @@ public:
 		// Force a virtual destructor in all subclasses:
 		virtual ~cCallback() {}
 		
-		/// Called when data is received from the remote party
+		/** Called when data is received from the remote party */
 		virtual void DataReceived(const char * a_Data, int a_Size) = 0;
 		
-		/// Called when data can be sent to remote party; the function is supposed to append outgoing data to a_Data
+		/** Called when data can be sent to remote party; the function is supposed to *append* outgoing data to a_Data */
 		virtual void GetOutgoingData(AString & a_Data) = 0;
 		
-		/// Called when the socket has been closed for any reason
+		/** Called when the socket has been closed for any reason */
 		virtual void SocketClosed(void) = 0;
 	} ;
 
@@ -78,24 +77,21 @@ public:
 	cSocketThreads(void);
 	~cSocketThreads();
 	
-	/// Add a (socket, client) pair for processing, data from a_Socket is to be sent to a_Client; returns true if successful
+	/** Add a (socket, client) pair for processing, data from a_Socket is to be sent to a_Client; returns true if successful */
 	bool AddClient(const cSocket & a_Socket, cCallback * a_Client);
 	
-	/// Remove the associated socket and the client from processing. The socket is left to send its data and is removed only after all its m_OutgoingData is sent
+	/** Remove the associated socket and the client from processing.
+	The socket is left to send its last outgoing data and is removed only after all its m_Outgoing is sent
+	and after the socket is properly shutdown (unless the remote disconnects before that)
+	*/
 	void RemoveClient(const cCallback * a_Client);
 	
-	/// Notify the thread responsible for a_Client that the client has something to write
+	/** Notify the thread responsible for a_Client that the client has something to write */
 	void NotifyWrite(const cCallback * a_Client);
 	
-	/// Puts a_Data into outgoing data queue for a_Client
+	/** Puts a_Data into outgoing data queue for a_Client */
 	void Write(const cCallback * a_Client, const AString & a_Data);
 	
-	/// Stops reading from the client - when this call returns, no more calls to the callbacks are made
-	void StopReading(const cCallback * a_Client);
-	
-	/// Queues the client for closing, as soon as its outgoing data is sent
-	void QueueClose(const cCallback * a_Client);
-
 private:
 
 	class cSocketThread :
@@ -114,13 +110,10 @@ private:
 
 		void AddClient   (const cSocket &   a_Socket, cCallback * a_Client);  // Takes ownership of the socket
 		bool RemoveClient(const cCallback * a_Client);  // Returns true if removed, false if not found
-		bool RemoveSocket(const cSocket *   a_Socket);  // Returns true if removed, false if not found
 		bool HasClient   (const cCallback * a_Client) const;
 		bool HasSocket   (const cSocket *   a_Socket) const;
 		bool NotifyWrite (const cCallback * a_Client);  // Returns true if client handled by this thread
 		bool Write       (const cCallback * a_Client, const AString & a_Data);  // Returns true if client handled by this thread
-		bool StopReading (const cCallback * a_Client);  // Returns true if client handled by this thread
-		bool QueueClose  (const cCallback * a_Client);  // Returns true if client handled by this thread
 		
 		bool Start(void);  // Hide the cIsThread's Start method, we need to provide our own startup to create the control socket
 		
@@ -134,24 +127,45 @@ private:
 		cSocket m_ControlSocket1;
 		cSocket m_ControlSocket2;
 		
-		// Socket-client-packetqueues triplets.
+		// Socket-client-dataqueues-state quadruplets.
 		// Manipulation with these assumes that the parent's m_CS is locked
 		struct sSlot
 		{
-			cSocket     m_Socket;  // The socket is primarily owned by this
+			/** The socket is primarily owned by this object */
+			cSocket m_Socket;
+			
+			/** The callback to call for events. May be NULL */
 			cCallback * m_Client;
-			AString     m_Outgoing;  // If sending writes only partial data, the rest is stored here for another send
-			bool        m_ShouldClose;  // If true, the socket is to be closed after sending all outgoing data
-			bool        m_ShouldCallClient;  // If true, the client callbacks are called. Set to false in StopReading()
+			
+			/** If sending writes only partial data, the rest is stored here for another send.
+			Also used when the slot is being removed to store the last batch of outgoing data. */
+			AString m_Outgoing;
+			
+			enum eState
+			{
+				ssNormal,          ///< Normal read / write operations
+				ssWritingRestOut,  ///< The client callback was removed, continue to send outgoing data
+				ssShuttingDown,    ///< The last outgoing data has been sent, the socket has called shutdown()
+				ssShuttingDown2,   ///< The shutdown has been done at least 1 thread loop ago (timeout detection)
+				ssRemoteClosed,    ///< The remote end has closed the connection (and we still have a client callback)
+			} m_State;
 		} ;
+		
 		sSlot m_Slots[MAX_SLOTS];
 		int   m_NumSlots;  // Number of slots actually used
 		
 		virtual void Execute(void) override;
 		
-		void PrepareSet     (fd_set * a_Set, cSocket::xSocket & a_Highest);  // Puts all sockets into the set, along with m_ControlSocket1
+		/** Puts all sockets into the set, along with m_ControlSocket1.
+		Only sockets that are able to send and receive data are put in the Set.
+		Is a_IsForWriting is true, the ssWritingRestOut sockets are added as well. */
+		void PrepareSet(fd_set * a_Set, cSocket::xSocket & a_Highest, bool a_IsForWriting);
+		
 		void ReadFromSockets(fd_set * a_Read);  // Reads from sockets indicated in a_Read
 		void WriteToSockets (fd_set * a_Write);  // Writes to sockets indicated in a_Write
+		
+		/** Removes those slots in ssShuttingDown2 state, sets those with ssShuttingDown state to ssShuttingDown2 */
+		void CleanUpShutSockets(void);
 	} ;
 	
 	typedef std::list<cSocketThread *> cSocketThreadList;
@@ -164,9 +178,3 @@ private:
 
 
 
-
-#endif  // CSOCKETTHREADS_H_INCLUDED
-
-
-
-