last day (24 days later) » 

3:21 AM
-1
Q: 'Connection timed-out' in the exact 10th minute of program run

GiannisI have written a C program that utilizes pthreads and TCP/IP communication in order to achieve the following: "Each node that runs the program produces some messages in a given frequency (seconds). When two nodes connect each node must update the other node's message list (buffer). The whole pro...

 
The buffer of each node fills with messages that each node creates in a random time interval and with messages that it receives from other nodes. I have excluded the relevant functions as they do not play any role on the problem.
 
Did you figure out which part of the code you need to change so that the error occurs at a different time?
 
I reckon it has to do with the sleep() functions maybe. On how they synchronize. Note that I experiment with exactly two nodes, that both start at the same time. The problem is I don't want it to happen at all. I want for my program to take care of this error and automate the return to normal communication.
 
In the subset of the code that's here, it looks like the server never closes connfd.
 
@MarkPlotnick But as you can see the connfd closing is outside my infinite for(;;) loop. So as my chat_server() function never breaks the loop, the close(sockfd) command never really executes, is it?
 
3:21 AM
It's also customary that after an accept, the server hands off processing of that descriptor to another thread. This can be done by forking, or creating a thread, or by sending the fd to another process. In all cases, the goal is that server shouldn't be doing something that prevents it from doing another accept.
 
@MarkPlotnick Thanks for your response. Isn't there any way that I can encounter the error without forking or opening another thread? I know that the sleep()'s that I use are a bit dorky but I don't think they are that kind of an issue
 
I guess you can have a single-threaded server use select on all its socket fd's, although I don't know if select can be used to let you know when a socket is ready to use accept.
 
@MarkPlotnick server has only one socket fd though
 
accept returns a socket fd, too.
Anyway, I suggest using the strace -f command to trace your server, and see if the accept starts to return -1 after awhile, or if the server gets preoccupied with a conversation with a client and neglects to do an accept while another client is trying to connect to it.
 
@MarkPlotnick Should I put it somewhere in my code? If yes, where? Or should I execute it while my program runs on a different shell?
 
3:21 AM
Try strace -f -e trace=network yourprogram to launch and trace your program.
 
You are never closing the accepted socket. You are ignoring the count returned by recv(). You are ignoring error results of socket(), setsockopt(), listen(), bind(), accept(), send(), and recv() This code isn't even complete enough to debug.
 
@user207421 I have on purpose excluded the error handling on this code in order for it to be readable-comprehensible. I think it's obvious that on my real program I have taken care of the error handling.
 
Yes, select can be used to determine whether a connection can be accept()ed from a listening socket without blocking.
No, @Giannis, it is not obvious that you have taken proper care of error handling in your real program. Indeed, error handling is exactly what the question seems to be about, so It seems apparent that the error handling is not fully correct.
 
The only thing that is obvious to me is that your code doesn't do what you think it does. Posting a different version of it with its own new errors is therefore futile. And you have ignored the other faults I mentioned in your code.
 
@user207421 I don't think you understand that I would not post a 500-line code here like it's nothing. The code you are witnessing here is a simplified version of the flow and it contains everything you need to know about my program. The error handling would just print some messages or just exit so it does not affect the core flow. So instead of being rude and offensive you could just give your advise and stop wasting the space you are given in the comments section.
 
3:21 AM
exactly which statement is causing the "Connection timed out" message?
running the posted code through the compiler results in LOTS of warning and ERROR messages. To start, this statement: ` setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &(int){ 1 }, sizeof(int);` is missing the final closing paren, ')'. Please post code that cleanly compiles
I see that both the server code and the client code are in the same file. This is a very poor architecture. Suggest splitting into two files, one for the server and one for the client.
the function: buffer_fill() allocates memory via a call to malloc(), but that memory is never passed too free(). The result is a memory leak that will soon cause the call to malloc() to fail. The posted code fails to check that the call to malloc() was successful. The statement: msg = MAX_MESSAGES-1;` sets the index into the allocated buffer to the last available location in the buffer, then msg ++; increments the index so the next call to buffer_fill() will again call malloc() Overall, a major memory leak (cont)
(cont) the statement: strcpy(buffer[msg], minima); is copying 'minima' (presumably a NUL terminated char string) in a area of memory using the pointer contained in buffer[msg] however, Those pointers are not pointing to anything specific, like memory that is owned by the application. This is an excellent example of Undefined Behavior
in function: chat_server(), regarding: char buff[MAX_LENGTH]; bzero(buff, MAX_LENGTH); It would be much better to simply write: char buff[ MAX_LENGTH ] = { '\0' };`
regarding: while(recv(sockfd, buff, sizeof(buff),0)>0){ ... buffer_fill(buff) The function recv() does not NUL terminate the read data. So in the function: buffer_fill() the statement: strcpy(buffer[msg], minima); will not find the NUL byte in the expected location. The result is Undefined Behavior. Suggest: `ssize_t byteCount; while( (byteCount = recv(sockfd, buff, sizeof(buff),0) ) >0){ buff[ byteCpunt ] = '\0'; ... buffer_fill(buff)
in function: chat_server(), The final statement; ` bzero(buff, MAX_LENGTH);` is useless as the next time the function is entered, this local array will be re-created/re-initialized
in function; server() regarding: for(;;){ connfd = accept(sockfd, (SA*)&cli, &len); chat_server(connfd); } There is no way to exit this loop! therefore, every connection will never go away. Most OSs can only handle a limited number of concurrent connections before aborting. Note: the server code can only receive messages, never re-distribute the messages to all the clients that are connected
regarding: usleep(0.5 * 1000000); the function usleep() is expecting a parameter of type: useconds_t however, the code is passing a parameter of type: double This is an error
 
 
4 hours later…
7:09 AM
@user3629249 on my buffer_fill() function problem: after msg++ the malloc() will not execute as the program will get into the else{ } statement, won't it?
:47500994on my strcpy(buffer[msg], minima) Undefined Behavior: how is that this pointers do not point to memory that is owned by my application? As you can see I have malloc()ed this space in my if{ } statement
 
7:29 AM
@user3629249 on my last 'bzero(buff, MAX_LENGTH);' statement: why is it useless. I want it to execute every time the while{ } executes
@user3629249 in this point I want to thank you so much for your help. Your advise has been proved so valuable to me.
 
8:32 AM
@user3629249 thank you for your comment on the fact that server does not redistribute the messages further. That's the purpose of my application. I want the server to receive messages and just save them in my buffer[]. So that's why I do not close my server. I have only two clients feeding my server with messages so the connections are not that many. So according to that, do you think there are still fixes to be made on that part of my code?
@user3629249 I understand. But why does my compiler accept these numbers? Shouldn't the function reject the arguments as their type is not compatible?
 
 
15 hours later…
11:29 PM
regarding my strcpy(buffer[msg], minima) Undefined Behavior: how is that this pointers do not point to memory that is owned by my application? As you can see I have malloc()ed this space in my if{ } statement the array of pointers is allocated, but not where each pointer points
regarding: on my last 'bzero(buff, MAX_LENGTH);' statement: why is it useless. I want it to execute every time the while{ } executes As I stated in my comment, every time the function is entered, the buffer is declared and set to all '0x00' AND it is a local buffer, on the stack, so when the function exits that buffer is completely gone. I.E. no need to re-initialize the buffer at the end of the function
 

  last day (24 days later) »