TCP/Transport: Fix tcp handling on win32.

1. Fix socket event (created by WSAEventCreate) handling for win32. This is to fix the issue which uncovered by FreeRDP#2750. We used to fall in loop after 2750. The reason is event created by WSAEventSelect on windows need manually reset, it is always in 'set' state even when we run out of the read buffer. It was first fixed in PR 2770. However the solution introduced another issue on Linux. This PR discards the fix in 2770 Later A quick fix for 2770 was introduced in 2790/2791. It is also discarded together. This fix try to keep usage same on Windows and Posix. 2. Fix argument for WSAEventSelect on win32 to be consistent with Linux. 3. Fix tcp event for listener.
realjiangms · Sep 5, 2015 · cafb2f7 · cafb2f7
1 parent ad4a862
commit cafb2f7
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 12 deletions.
diff --git a/libfreerdp/core/listener.c b/libfreerdp/core/listener.c
@@ -168,11 +168,28 @@ static BOOL freerdp_listener_open(freerdp_listener* instance, const char* bind_a
 			continue;
 		}
 
-		/* FIXME: these file descriptors do not work on Windows */
-
 		listener->sockfds[listener->num_sockfds] = sockfd;
+#ifdef _WIN32
+		listener->events[listener->num_sockfds] = WSACreateEvent();
+
+		if (listener->events[listener->num_sockfds])
+		{
+			/* WSAEventSelect automatically sets the socket in non-blocking mode */
+			/* Set lNetworkEvents as same behavior as being put in readfds in select()
+			 * to keep same behavior between WIN32 and posix.
+			 * From MSDN for function select:
+			 * https://msdn.microsoft.com/en-us/library/windows/desktop/ms740141(v=vs.85).aspx
+			 * In summary, a socket will be identified in a particular set when select returns if:
+			 * readfds:
+			 * 1. If listen has been called and a connection is pending, accept will succeed.
+			 * 2. Data is available for reading (includes OOB data if SO_OOBINLINE is enabled).
+			 * 3. Connection has been closed/reset/terminated. */
+			WSAEventSelect(sockfd, listener->events[listener->num_sockfds], FD_READ | FD_ACCEPT | FD_CLOSE);
+		}
+#else
 		listener->events[listener->num_sockfds] =
 			CreateFileDescriptorEvent(NULL, FALSE, FALSE, sockfd, WINPR_FD_READ);
+#endif
 		listener->num_sockfds++;
 
 		WLog_INFO(TAG, "Listening on %s:%s", addr, servname);
@@ -336,7 +353,7 @@ static BOOL freerdp_listener_check_fds(freerdp_listener* instance)
 	void* sin_addr;
 	int peer_sockfd;
 	freerdp_peer* client = NULL;
-	socklen_t peer_addr_size;
+	int peer_addr_size;
 	struct sockaddr_storage peer_addr;
 	rdpListener* listener = (rdpListener*) instance->listener;
 	static const BYTE localhost6_bytes[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 };
@@ -347,8 +364,18 @@ static BOOL freerdp_listener_check_fds(freerdp_listener* instance)
 
 	for (i = 0; i < listener->num_sockfds; i++)
 	{
+#ifdef _WIN32
+		/* Manually reset the event created by WSACreateEvent.
+		 * It will be reenabled by accept if there is still data to be read */
+		WSAResetEvent(listener->events[i]);
+#else
+		/* Don't need to reset event for posix socket. The socket event is bound 
+		 * to the socket fd itself. So it is automatically 'reseted' if there is
+		 * no more data to be read */
+#endif
+
 		peer_addr_size = sizeof(peer_addr);
-		peer_sockfd = accept(listener->sockfds[i], (struct sockaddr*) &peer_addr, &peer_addr_size);
+		peer_sockfd = _accept(listener->sockfds[i], (struct sockaddr*) &peer_addr, &peer_addr_size);
 		peer_accepted = FALSE;
 
 		if (peer_sockfd == -1)

diff --git a/libfreerdp/core/tcp.c b/libfreerdp/core/tcp.c
@@ -145,6 +145,16 @@ static int transport_bio_simple_read(BIO* bio, char* buf, int size)
 
 	BIO_clear_flags(bio, BIO_FLAGS_READ);
 
+#ifdef _WIN32
+	/* Manually reset the event created by WSACreateEvent.
+	 * It will be reenabled by recv if there is still data to be read */
+	WSAResetEvent(ptr->hEvent);
+#else
+	/* Don't need to reset event for posix socket. The socket event is bound 
+	 * to the socket fd itself. So it is automatically 'reseted' if there is
+	 * no more data to be read */
+#endif
+
 	status = _recv(ptr->socket, buf, size, 0);
 
 	if (status > 0)
@@ -366,7 +376,16 @@ static int transport_bio_simple_init(BIO* bio, SOCKET socket, int shutdown)
 			return 0;
 
 		/* WSAEventSelect automatically sets the socket in non-blocking mode */
-		WSAEventSelect(ptr->socket, ptr->hEvent, FD_READ | FD_WRITE | FD_CLOSE);
+		/* Set lNetworkEvents as same behavior as being put in readfds in select()
+		 * to keep same behavior between WIN32 and posix.
+		 * From MSDN for function select:
+		 * https://msdn.microsoft.com/en-us/library/windows/desktop/ms740141(v=vs.85).aspx
+		 * In summary, a socket will be identified in a particular set when select returns if:
+		 * readfds:
+		 * 1. If listen has been called and a connection is pending, accept will succeed.
+		 * 2. Data is available for reading (includes OOB data if SO_OOBINLINE is enabled).
+		 * 3. Connection has been closed/reset/terminated. */
+		WSAEventSelect(ptr->socket, ptr->hEvent, FD_READ | FD_ACCEPT | FD_CLOSE);
 #else
 		ptr->hEvent = CreateFileDescriptorEvent(NULL, FALSE, FALSE, (int) ptr->socket, WINPR_FD_READ);
 

diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c
@@ -745,24 +745,17 @@ int transport_check_fds(rdpTransport* transport)
 	int status;
 	int recv_status;
 	wStream* received;
-	HANDLE event;
 
 	if (!transport)
 		return -1;
 
-	if (BIO_get_event(transport->frontBio, &event) != 1)
-		return -1;
-
 	/**
 	 * Loop through and read all available PDUs.  Since multiple
 	 * PDUs can exist, it's important to deliver them all before
 	 * returning.  Otherwise we run the risk of having a thread
 	 * wait for a socket to get signaled that data is available
 	 * (which may never happen).
 	 */
-#ifdef _WIN32
-	ResetEvent(event);
-#endif
 	for (;;)
 	{
 		/**