[SOLVED] Blocking threads in C

#include <sys/socket.h>
#include <sys/types.h>
#include <arpa/inet.h>
#include <pthread.h>

struct arguments
		{
			int *new_sock;
			int port;
			char *IP;
		};
pthread_t cli[1000];

int connection();
int *cli_handler(void *arg);

int main( int argv, char* const argc[] )
{
	connection();
	return 0;
}

int connection()
{
	int sock_fd,
		cli_sock,
		len,
		PORT = 4444;
	struct sockaddr_in server;

	server.sin_family = AF_INET;
	server.sin_port = htons(PORT);
	server.sin_addr.s_addr = INADDR_ANY;

	/* bind
	 * listen
	 * accept
	 */

	sock_fd = socket( AF_INET, SOCK_STREAM, 0);

	if( sock_fd == -1 )
	{
		perror("SOCKET FAILED!!!");
		return 1;
	}
	puts("SOCKET CREATED");

	if( bind( sock_fd, (struct sockaddr*)&server, sizeof(server) ) == -1 )
	{
		perror("BIND FAILED!!!");
		return 1;
	}
	puts("BIND DONE");

	listen(sock_fd, 1000);
	//why does printf not output text here? but no errors are given
	puts("LISTENING ON PORT 4444....");

	len = sizeof(struct sockaddr_in);
	int index00;

	struct arguments *ptr_arg;

point00:while( cli_sock = accept (sock_fd, (struct sockaddr  *) &server, (socklen_t *)&len) )
		{
			if(cli_sock == -1)
			{
				perror("ACCEPT FAILED!!!");
				return 1;
			}
			ptr_arg = (struct arguments *)malloc(sizeof(*ptr_arg));
			ptr_arg->new_sock = cli_sock;
			printf( "Connection received from: %s %d\n", inet_ntoa(server.sin_addr), ntohs(server.sin_port) );

			ptr_arg->IP = inet_ntoa(server.sin_addr);
			ptr_arg->port = ntohs(server.sin_port);

			//why would someone put < 0 here

			for( index00=0; index00<1000; index00++ )
			{
				if( cli[index00] == NULL )
				{
					if( pthread_create( &cli[index00], NULL, cli_handler, (void *)ptr_arg ) < 0 )
					{
						perror("THREAD FAILED");
						return 1;
					}
					pthread_join(cli[index00], NULL);
					goto point00;
				}
				index00++;
			}
		}
}

int *cli_handler(void *arg)
{
	int new_sock,// = *(int *)sock,
		write_fd,
		recv_fd,
			i;

	char buff[256];

	struct arguments *ptr_arg;
	ptr_arg = (struct arguments *)arg;

	new_sock = ptr_arg->new_sock;
	int port_f = ptr_arg->port;

	int len = strlen(ptr_arg->IP);
	char test[len];
	for( i=0; i<len; i++ )
		test[i] = ptr_arg->IP[i];

	while(1)
	{
		write_fd = write( new_sock, "TEST: ", 6 );
		if( write_fd == 0 )
		{
			puts("NOTHING WRITTEN");
		}
		else if( write_fd == -1 )
		{
			perror("\nFAILED TO WRITE");
			return 1;
		}

		if ( ( recv_fd = recv( new_sock, buff, 256, 0 ) ) == -1 )
		{
			perror("\nRECV FAILED");
			return 1;
		}

		printf("FROM %s %d: %s", test, port_f, buff);

		for( i=0; i<256; i++ )
			buff[i] = '\0';
	}
	free(arg);
}


//CLINET IP IDENTIFIER DONE

//server input to send to specific client
//TThread index managing

Code is above, the problem is following.
Multiple clients may connect, but only one at a time may send messages to the server, when one discconects next one may send and so on.
Can anyone tell me as to why is this happening, how to fix it.
I think that threads are somehow blocking one another but I am not sure.

pthread_join waits for thread termination

Ok so I just went ahead and did pthread_detach that seamed to have solved the problem, I ll just leave this open for a bit longer to see if anyone will maybe give some other suggestions and than mark it as solved.

Thank you very much for your help.

Make sure that you have some sane limit on the number of threads and resources used. It sounds dangerous just creating a thread for every client with no obvious limit. There are default limits, but they are hit in an unknown order (resource limit may be hit before number of threads or vice versa).

Make sure to read:
http://linux.die.net/man/3/pthread_create

You should treat "threads" as employees, not hamburgers. If a bus-load of 80 very hungry people suddenly walk into a McDonald's restaurant somewhere, the number of employees on duty will not magically increase ... nor will all any of those 80 hungry people crowd-around in the kitchen! Carefully observe how that restaurant actually works, and do the same in your software.

Not sure what you mean, there is a limit or so I hope, I ve set 1000 as pthread_t cli[1000]; and the for loop goes to 1000, it is too much now, but I ve jsut set a high number now so I do not have to think about it, I am not expecting more then 50 or so.

---------- Post added 07-29-14 at 06:15 AM ----------

OS is Debian jessie, with 8GB ram and i7

How about, say, fifty threads, instead? Or twenty? Or ten?

How many "units of work" can this system reliably and predictably achieve, in parallel? Find out that number, and set that number of threads.

Represent the "units of work" as records on one-or-more queues ... just like the little pieces of paper that are filled-out by the waitress and stuck onto various carousels in the kitchen, and which finally accompany the finished order just before said piece of paper is punched onto a spindle and the tasty dish is carried to your table.

If a burger takes, say, three minutes to cook, and fifty of them are being cooked at once, you will not see fifty employees standing next to the grill, each of them frozen-in-place like little statues, waiting for "their" burger, or cooking it. No, there's just one guy standing there in front of the grill, and he's simultaneously observing all the burgers and doing whatever needs to be done, when it needs to be done, in order to custom-prepare every one. Design your system that way.

A well-tuned restaurant with five people in the kitchen can turn out several hundred orders per hour. No one's allowed in the kitchen who doesn't have a job to do there. The workflow is designed so that it "scales-up" well, even when a busload of seniors turns up at the front door.

Multiple clients may connect, but only one at a time may send messages to the server, when one discconects next one may send and so on.

1. The is caused by the pthread_join() embedded inside the accept while loop. pthread_join waits for the thread to complete its work and exit.
2. Once the pthread_join is removed you will only be able to accept 1000 connections, because 'pthread_t cli[1000];' is the effective connection limit.
3. Allow the cli_handler() thread service routine to exit properly.

Can anyone tell me as to why is this happening, how to fix it.

1. Remove the pthread_join from the accept while loop. (Also remove the 'goto')
2. Rethink the 'pthread_t cli[1000];' control.
3. Use 'pthread_exit(NULL);' right after 'free(arg)'.

I think that threads are somehow blocking one another but I am not sure.

• No, this is not it at all; pthread_join() is your issue, and pthread_detach() is the wrong thing to do.

I liked the employee/hamburger statement. I would redo the accept loop and the cli_handler to match the employee/hamburger pattern.

• Employees relates to the total number of threads available. I would strongly suggest a max number of 10 or less.
• However, the time it takes to make a hamburger is an important factor. I've noticed cli_handler has a while(true) loop that reads messages forever. Think about how many messages the average client will send on each connection? if the answer is 1 then the number of employees/threads could be 4. However, if the number of messages sent is 50+ then the number of employee/threads may need to be 10. If the number of messages sent is unlimited, or once a connection is made it never drops; then you will need an employee/thread for each client -- and if that number would be greater than 200, you will need a totally different approach i.e the employee/hamburger pattern is no longer valid.
• Create x number of threads first -- or devise an activeThreadCount counter thats incremented when threads are created, and decrements when threads exit.
• Make cli_handler() reentrant, so that it never exits, if threads are created first.
• Figure out how to send work(connection/ptr_arg) to one of these pre-created threads, maybe expand 'ptr_arg'; or have them read a (ptr_arg) from a blocking queue.

There are lots of ways to design a multi-threading module. To be more specific,I would need to understand more of your whole objective.

skoona

I would have to agree that you should think about redoing the code. The current solution will lead to problems.