Advertisement

Keep receiving old data after closing socket

Started by November 18, 2018 10:35 PM
3 comments, last by hplus0603 6 years ago

I'm trying to write a simple server application using Winsock2. I have a socket listening on port 80 (for http traffic). When I run the program and use my browser to connect to 127.0.0.1, the server receives the connection and displays it as expected.


New Connection

Recieved message:
GET / HTTP/1.1
Host: 127.0.0.1
Connection: keep-alive
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36
DNT: 1
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9

The problem comes when I try to close the socket. I'm using the following two commands: 


closesocket(currentSocket);
FD_CLR(currentSocket, &socketList);

When I do this, the program keeps repeating the output as if it was getting the same request over and over. If I remove either of these commands it only displays the output once, as it should.

The complete source code is as follows. This is a simplified version of the code that will duplicate the behavior, so I've omitted certain checks for clarity.


#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif // !WIN32_LEAN_AND_MEAN

#pragma comment(lib, "ws2_32.lib")
#include <WinSock2.h>

#include <iostream>

void runTest()
{
	////// SETUP ///////
	const int LISTENING_PORT = 80;
	const int BUFFER_SIZE = 4098;

	WSADATA wsa;
	sockaddr_in listeningAddress;
	listeningAddress.sin_family = AF_INET;
	listeningAddress.sin_port = htons(LISTENING_PORT);
	listeningAddress.sin_addr.S_un.S_addr = INADDR_ANY;

	WSAStartup(MAKEWORD(2, 2), &wsa);
	SOCKET listeningSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
	bind(listeningSocket, (sockaddr*)&listeningAddress, sizeof(listeningAddress));
	listen(listeningSocket, SOMAXCONN);

	fd_set socketList;
	FD_ZERO(&socketList);
	FD_SET(listeningSocket, &socketList);

	////// MAIN LOOP ///////////
	bool isRunning = true;
	while(isRunning)	// endless loop, normally has interupt mechanism
	{
		fd_set socketListCopy = socketList;
		int socketCount = select(0, &socketListCopy, nullptr, nullptr, nullptr);

		for (int i = 0; i < socketCount; i++)
		{
			SOCKET currentSocket = socketListCopy.fd_array[i];
			if (currentSocket == listeningSocket)
			{
				std::cout << "New Connection\n";
				SOCKET client = accept(listeningSocket, nullptr, nullptr);
				FD_SET(client, &socketList);
			}
			else
			{
				char buffer[BUFFER_SIZE];
				memset(buffer, 0, BUFFER_SIZE);
				int bytesIn = recv(currentSocket, buffer, BUFFER_SIZE, 0);

				std::cout << "\nRecieved message:\n" << buffer << '\n';

				// Using the following two lines causes the output to loop
				closesocket(currentSocket);
				FD_CLR(currentSocket, &socketList);
			}
		}
	}
	WSACleanup();
}

 

First: The fd_set is a bitmask on any operating system except Windows (where sockets are handles, not file numbers.) Thus, "fd_array" is not a thing on other operating systems. Generally, you will want to keep the list of sockets in a separate container (like a map<SOCKET, your-data>) and create the fd_set for receive/send/exception from zero before each call to select().

Second: You're not checking errors in calls to closesocket() or recv(). Also, if you set a breakpoint on closesocket(), and then step through the loop in the debugger, what is the behavior of the program? I bet this will tell you what's going on.

Third: Do you know what the web browser is doing? Because if you close the socket without sending anything, maybe it will re-try? If that is the problem, then try calling


char const *errmsg = "HTTP/1.0 500 Not Implemented\r\n\r\n";
error = send(currentSocket, errmsg, strlen(errmsg), 0);

to make sure the browser gets an answer.

 

enum Bool { True, False, FileNotFound };
Advertisement

Thanks for the help. You were right, it was the browser.

I just want to clarify the first point; If I understand correctly you just need a different place or structure to hold the sockets, but you can still use select to call them the same as before?

As for the second point, I did have error checking in my original code, I just left it out of my example code to keep it smaller and easier to read... maybe I left out a bit too much.

you just need a different place or structure to hold the sockets, but you can still use select to call them the same as before?

Yes, you will typically build something like a std::map<SOCKET, shared_ptr<YourClientInfo>> to keep track of which sockets are "live." The YourClientInfo pointer is where you hang off all the information that the clients will provide through the session.

Each time you call select(), you will typicall walk through your list of clients, and the listening socket, and add them to a fresh (empty) fd_set, before calling select(). Beware that, on Windows, you can only add 64 total sockets to a fd_set; on Linux the typical limitation is 1024. This may be possible to increase by defining NFDS, but once you have more sockets than work by default, you'll typically want to use a better API than select() anyway. libevent on Linux, and I/O completion ports with OVERLAPPED I/O on Windows. You can use boost::asio as a wrapper around both of those methods in a platform-independent way.

 

enum Bool { True, False, FileNotFound };

This topic is closed to new replies.

Advertisement