accept() returns 0

[Guest42]

I'm am trying to learn about sockets by writing a simple chat client. Basically, multiple users will be able to connect to a server which will relay the messages back to each user

At the moment it does not relay messages back. This is because connecting multiple users seems to cause a problem. Basically, the parent fork calls accept while children forks handle each client.
The problem arrises when accept returns 0. This happens randomly on the second or third call to accept(). When this happens, that socket does not work. Any calls to accept afterwards also return 0. And the socket most recently succesfully created will 'cause a bad file descriptor error next time the client tries to message. Is this a bug with accept()? Here is the code for the host:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/shm.h>
#include <netinet/in.h>

#define PORT 4230
#define MAX_SIZE 100
#define MAX_CLIENTS 10

void progError(const char *err)
{
	perror(err);
	exit(1);
}

typedef struct
{
	int noClients;
	int clientSocket[MAX_CLIENTS];
	struct sockaddr_in clientAddress[MAX_CLIENTS];
	unsigned int clientAddressSize[MAX_CLIENTS];
} ClientInfo;

int localSocket;
struct sockaddr_in localAddress;

int clientID;
char in_message[MAX_SIZE];
pid_t parentPID;

int shared_id;
ClientInfo *sharedInfo;

int isRunning;
int i;

int main(void)
{
	shared_id = shmget(IPC_PRIVATE, (sizeof(ClientInfo)), 0666);
	if(shared_id > 0)
		sharedInfo = (ClientInfo *)shmat(shared_id, (void *)0, 0666);
	else
		progError("Can't allocate memory");

	sharedInfo->noClients = 0;
	parentPID = getpid();
	isRunning = 1;

	localSocket = socket(AF_INET, SOCK_STREAM, 0);
	localAddress.sin_family = AF_INET;
	localAddress.sin_port = htons(PORT);
	localAddress.sin_addr.s_addr = INADDR_ANY;

	if(bind(localSocket, (struct sockaddr*)&localAddress, sizeof(struct sockaddr_in)) == -1)
		progError("Couldn't bind socket");

	if(listen(localSocket, 5) == -1)
		progError("Server is a bad listener");
	else
		printf("Waiting for client to connect\n");

	while(isRunning == 1)
	{
		if(getpid() == parentPID)
		{
			if((sharedInfo->clientSocket[sharedInfo->noClients] = accept(localSocket, (struct sockaddr*)&sharedInfo->clientAddress[sharedInfo->noClients], &sharedInfo->clientAddressSize[sharedInfo->noClients])) == -1)
			{
				progError("Server can't accept that");
			}
			else if(sharedInfo->clientSocket[sharedInfo->noClients] == 0)
			{
				printf("Client returns 0\n");
				close(sharedInfo->clientSocket[sharedInfo->noClients]);
			}
			else
			{
				pid_t return_pid = fork();
				switch(return_pid)
				{
					case -1:
						progError("Could not create client thread");
					case 0:
						printf("Client thread created: %d\n", sharedInfo->clientSocket[sharedInfo->noClients]);
						clientID = sharedInfo->noClients;
						++sharedInfo->noClients;
						break;
					default:
						printf("This is the parent thread\n");
				}
			}
		}
		else
		{
			int n;
			bzero(&in_message, strlen(in_message));
			
			if((n = recv(sharedInfo->clientSocket[clientID], &in_message, MAX_SIZE, 0)) == -1)
				progError("Couldn't recieve message");
			else
				printf("Client %d says - %d: %s", clientID, n, in_message);

			if(in_message != 0)
				for(i = 0; i < sharedInfo->noClients; i++)
				{
					printf("This should print when the message is there\n");
				}
		}
	}

	return 0;
}

And the client:

#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <limits.h>

#define HOST_NAME "piratebox"
#define DEST_PORT 4230
#define MAX_SIZE 100

int localSocket, hostSocket;
struct sockaddr_in hostAddress;

char in_message[LINE_MAX];
int n;

int main(void)
{
	localSocket =  socket(AF_INET, SOCK_STREAM, 0);

	hostAddress.sin_family = AF_INET;
	hostAddress.sin_port = htons(4230);
	hostAddress.sin_addr.s_addr = inet_addr("127.0.0.1");

	//could optionally bind local socket, but if not
	//kernel chooses port for us

	if( connect(localSocket, (struct sockaddr*)&hostAddress, sizeof(struct sockaddr)) == -1)
		perror("Can't connect to client");

	
	while(1)
	{
		printf("Send a message to the server:\n");
		
		fgets(in_message, LINE_MAX, stdin);
	
		if(send(localSocket, in_message, strlen(in_message), 0) == -1)
		perror("Couldn't send message");

		bzero(in_message, strlen(in_message));
	}
	
	close(localSocket);
	printf("End of program\n");

	return 0;
}

 

[Guest42]

Whoops. I might add that, if instead of closing the '0'ed socket, I let it try to recieve a message, I get the "Socket operation on non Socket" error, which I suppose means 0 is reserved for something else.

 

[rburke]

My guess is that the blocking "accept" is getting interrupted by another signal. While that usually returns -1 you might want to change the "printf" in the accept returns 0 to "perror" and see what the errno is be set to. like this:

Yours:

            else if(sharedInfo->clientSocket[sharedInfo->noClients] == 0)
            {
                printf("Client returns 0");
                close(sharedInfo->clientSocket[sharedInfo->noClients]);
            }

New:

else if(sharedInfo->clientSocket[sharedInfo->noClients] == 0)
            {
                perror("Accept call returns 0\n");
                close(sharedInfo->clientSocket[sharedInfo->noClients]);
            }

 

[Salem]

> Is this a bug with accept()?
probably not

But I do see lots of potential bugs in your code.

Like all the wonderful race conditions over the unprotected access to all your shared data.

>

if((sharedInfo->clientSocket[sharedInfo->noClients] = accept(localSocket, (struct sockaddr*)&sharedInfo->clientAddress[sharedInfo->noClients], &sharedInfo->clientAddressSize[sharedInfo->noClients])) == -1)

Simplify it man - this is just embedded assignment gone mad.
How can anyone read that?

> if(send(localSocket, in_message, strlen(in_message), 0)
send() may also successfully send part of the message.
In fact send() and recv() can both deliver part of the message. You need to do your own recovery of message fragmentation.

> if(in_message != 0)
in_message is an array, so it's address is both fixed at compile time, and never NULL. This is a pointless comparison.

--