Server crash when client closed socket

lrios · 05-10-2008, 03:49 PM

Hi,
I am trying that the server dont crash when any client closed the socket.
The server must be always alive, but itn't work.

the client send data to server, and when the client send "/q" or "/quit" the conection will be closed.

The client only send data.
#client

Code:

while(1) 
	{
	memset(buffer_c, 0, sizeof(buffer_c));	
	printf("Message to send -> ");
	fgets(buffer_c, sizeof(buffer_c), stdin);


		if ((strcmp ("/help\n", buffer_c) == 0) || (strcmp ("/h\n", buffer_c) == 0)) 
		{
			
			printf("Help ... /quit /q \n");
		}	

		else if ((strcmp("/quit\n", buffer_c) == 0) ||  (strcmp("/q\n", buffer_c) == 0)) 
		{	

                      printf("close socket\n");
                               close(sockfd);
                     
                 }
                                
		n = write(sockfd, buffer_c, strlen(buffer_c)+1);
    			
      				if (n <= 0) {
					printf("error in write()");
					close(sockfd);
				exit(-1);
				}
    	
	printf("Mensaje Ok\n");
}

the server olny receive data.
#server

Code:

	while (1) {
	
	memset(buffer_s, 0, sizeof(buffer_s));	
	n = read(net, buffer_s, sizeof(buffer_s));
	if (n <= 0) {
	
		printf("error en read()\n");		
		write(net, "message isnt ok", 38);
	        return 0; 
	}

	printf("message received: %s", buffer_s);
}

paulsm4 · 05-10-2008, 10:03 PM

1. Please post the exact error message with the crash
<= A "crash" can be any of several different things

2. Please try to find out where in your code the crash is occurring\
<= A debugger (e.g. "gdb") can help with both 1) and 2)

3. Please post where "buffer_s" is declared and allocated in the server
<= It's very likely the "crash" is a segmentation violation...
... and also likely that the cause is a memory allocation error

4. Consider removing the funky "\n" from your string literals

Code:


  for ( ;; )
  {
    /* don't really need this: memset(buffer_c, 0, sizeof(buffer_c)); */
    printf("Message to send -> ");
    fgets(buffer_c, sizeof(buffer_c), stdin);
    buffer_c[strlen (buffer_c)-1] = '\0';
    if ((strcmp ("/help", buffer_c) == 0) || (strcmp ("/h", buffer_c) == 0))
    {
      printf("Help ... /quit /q \n");
    }	
    else if ((strcmp("/quit", buffer_c) == 0) ||  (strcmp("/q", buffer_c) == 0)) 
    {
      printf("close socket\n");
      close(sockfd);
    }
    ...

'Hope that helps .. PSM

Wim Sturkenboom · 05-11-2008, 07:14 AM

In the server:
If read returns 0 (zero), it's not an error but an indication that the socket is closed. And next you try to send a message back to the client on that closed socket.

Further I agree with paulsm4's comments.

With regards to debugging: simple printf's (with flushing of stdout after that) will already help you a lot.

paulsm4 · 05-11-2008, 10:26 AM

Hi, Wim -

One addendum - in the past, I've not always been able to rely on "printf" and "fflush". Most of the time it worked fine ... and sometimes it did't ... and the times it didn't were usually the times that mattered most.

So instead, I've gotten into the habit of simply using "fprintf (stderr, ...)" (the reason, of course, is that stdout is buffered and might need a "flush", but stderr isn't, and never needs to be flushed).

EXAMPLE:

Code:

  fprintf (stderr, "DBG: reading from socket...\n");
  iret = recv (s, buff, sizeof (buff), 0);
  fprintf (stderr, "DBG: read complete, iret = %d\n", iret);
  ...

IMHO .. PSM

PS:
Do you have any idea what this "crash" lrios is experiencing might be? The last code he posted that would have caused a crash was when he did an "strcpy" into a string constant. I haven't seen anything recently that would cause a "program abort" kind of crash...

Wim Sturkenboom · 05-11-2008, 12:33 PM

Hi Paul,

I'm not sure but my feeling is that it's in the write when a read error occures (it's 3 years ago that I last used C). I expect either a segmentation fault or a broken pipe (can't remember if the latter actually crashes a program).

Ad1
He's trying to write 38 bytes on error, but the string is shorter; therefore it possible that he accesses memory that is not 'his' (although it's only a read action); I don't know how compilers handle fixed strings (where they are stored), so it's an assumption.
Ad2
See my previous post (write over a broken pipe)

Thanks for your suggestion about fprintf(stderr,...); never thought about that. I learned the 'hard' way that printf on it's own does not do the trick (specially with segmentation faults) and that I had to add the flush.
If I ever write a C program again (it's currently not in my line of work), I will keep it in mind and maybe I will try to get a bit more into gdb.

lrios · 05-11-2008, 03:20 PM

This line doesnt work at all.

Code:

buffer_c[strlen (buffer_c)-1] = '\0';

by the way, the server doesnt crash, i mean when the server received "/q" or "/quit" stringbuffer, the server closed the conection and i dont want it.

buffer_s is declared as

Code:

char buffer_s[256]

paulsm4 · 05-11-2008, 06:06 PM

Yes, it does work:

Code:

#include <stdio.h>

int
main (int argc, char *argv[])
{
  char buff[256];
  printf ("Enter something: ");
  fgets (buff, sizeof (buff), stdin);
  buff[strlen (buff)-1] = '\0';
  printf ("You entered: %s...\n", buff);
  return 0;
}

Please compile and run this code. You should see something like this:

Quote:

Enter something: abc
You entered: abc...

Q: Why do you think it might be a Good Idea for "fgets()" to return the new line in a string (even though it makes you go to the extra effort of getting rid of it, like we did above)?

Q: Why do you think it's a Good Idea to transmit the null byte ("strlen()+1") when you send a variable length string over a socket?

Please read these two links (if you haven't already):
http://www.tenouk.com/Module39.html
http://beej.us/guide/bgnet/output/ht....html#lowlevel

lrios · 05-11-2008, 08:52 PM

paul,

1)the server closed the conection when received /q or /quit. I was wrong when i said crash, sorry about that.

2)
if i remove \n it doesnt works.

Code:

if ((strcmp ("/help\n", buffer_c) == 0) || (strcmp ("/h\n", buffer_c) == 0)) 
		{
			
			printf("Help ... /quit /q \n");
		}	

		else if ((strcmp("/quit\n", buffer_c) == 0) ||  (strcmp("/q\n", buffer_c) == 0)) 
		{	

                      printf("close socket\n");
                               close(sockfd);
                     
                 }

With this line can i remove the null character ? '\0'?

Code:

buffer_c[strlen (buffer_c)-1] = '\0';

3)the best solucion will be eliminated \n in "/quit\n" AND "/q\n", and my server dont closed the conection

I read those two links!

Wim Sturkenboom · 05-11-2008, 11:07 PM

Paul's suggestion (and mine in another post) is to use buffer_c[strlen (buffer_c)-1] = '\0'. In that case however you must compare with /quit and not with /quit\n.

Code:

#include <stdio.h>

int
main (int argc, char *argv[])
{
  char buff[256];
  printf ("Enter something: ");
  fgets (buff, sizeof (buff), stdin);
  printf ("You entered: >%s<\n", buff);
  buff[strlen (buff)-1] = '\0';
  printf ("You entered: >%s<\n", buff);
  return 0;
}

This is a slightly modified version of paul's code in post #7. Compile this and notice the difference that the two printf's will give you; the first one will include a newline (the '<' will be on a new line).

See man fgets why you get the newline in your buffer.

lrios · 05-12-2008, 09:15 AM

I think that buffer_c[strlen(buffer_c)-1]='\0' makeme troublemaker

#client

Code:

while(1) 
	{
	printf("Message to send -> ");
	fgets(buffer_c, sizeof(buffer_c), stdin);
	buffer_c[strlen(buffer_c)-1]='\0'; 

 
		if (((strcmp ("/help", buffer_c) == 0)) || ((strcmp ("/h", buffer_c) == 0))) 
		{

			
				printf("Help ... \n");
		}	
		else if (strcmp("/quit", buffer_c) == 0) 
		{	
                      printf("closed\n");
                               close(sockfd);
                     
                 }
                                
		n = write(sockfd, buffer_c, strlen(buffer_c));
    			
      				if (n <= 0) {
					printf("error in write()");
					close(sockfd);
				exit(-1);
				}
    	
	printf("Mensaje Ok\n");
}

#sever

Code:

	while (1) {
	
	n = read(newsocket, buffer_s, sizeof(buffer_s));
	if (n <= 0) {
	
		printf("error en read()\n");		
		write(newsocket, "Error", 6);
		exit(-1);
	}

	printf("message received: %s", buffer_s);
}

When I added "buffer_c[strlen(buffer_c)-1]='\0'"
Now the server dont received any message

paulsm4 · 05-12-2008, 12:42 PM

Please refer to the above post:

Quote:

Q: Why do you think it might be a Good Idea for "fgets()" to return the new line in a string (even though it makes you go to the extra effort of getting rid of it, like we did above)?

Q: Why do you think it's a Good Idea to transmit the null byte ("strlen()+1") when you send a variable length string over a socket?

1. Please compile and run Wim's sample code:

Code:

#include <stdio.h>

int
main (int argc, char *argv[])
{
  char buff[256];
  printf ("Enter something: ");
  fgets (buff, sizeof (buff), stdin);
  printf ("You entered: >%s<\n", buff);
  buff[strlen (buff)-1] = '\0';
  printf ("You entered: >%s<\n", buff);
  return 0;
}

QUESTION: what does "buff[strlen (buff)-1] = '\0';" do?

2. Please compare these two code snippets:

Code:

    n = write(sockfd, buffer_c, strlen(buffer_c));
    			
      				if (n <= 0) {
					printf("error in write()");
					close(sockfd);
				exit(-1);
				}

Code:

    n = write(sockfd, buffer_c, strlen(buffer_c)+1);			
    if (n < 0) 
    {
      perror ("socket write error");
      close(sockfd);
      exit(-1);
    }

QUESTIONS: What differences do you see? Why do you think "strlen()+1" is important?

lrios · 05-12-2008, 03:20 PM

I compile and run Wim's sample code and i see the differences, how important is buffer_c[strlen(buffer_c)-1]='\0'.

I repeat: The client send (datastring) buffer_c, but the server dont received them. the code is in the thread before.

thx

Wim Sturkenboom · 05-13-2008, 02:26 AM

First of all, your server does receive the message !!

How do I know? I've added a simple printf after the read

Code:

printf("%d bytes received\n",n);

You think that nothing has been received because printf uses buffered IO and will not always print immediately. You can see this if you send a message (e.g. hallo) and next send a /quit. The hallo will then be printed as the server terminates.
If you add a '\n' to the last printf in the server while loop, it will print the received message.

A note:
be aware that you might see some funnie stuff because of missing string terminators at the server side.

my client

Code:

        while(1)
        {
                printf("Message to send -> ");
                fgets(buffer_c, sizeof(buffer_c), stdin);
                printf("before: '%s'\n",buffer_c);
                buffer_c[strlen(buffer_c)-1]='\0';
                printf("after : '%s'\n",buffer_c);


                if (((strcmp ("/help", buffer_c) == 0)) || ((strcmp ("/h", buffer_c) == 0)))
                {
                        printf("Help ... \n");
                }
                else if (strcmp("/quit", buffer_c) == 0)
                {
                      printf("closed\n");
                      close(sockfd);
                }

                n = write(sockfd, buffer_c, strlen(buffer_c));

                if (n <= 0)
                {
                        printf("error in write()");
                        close(sockfd);
                        exit(-1);
                }

                printf("Message Ok\n");
        }

I've only added the printf's after the fgets and after the removal of the terminator for debugging.

my server

Code:

        while (1)
        {
                memset(buffer_s,0,sizeof(buffer_s));
                n = read(clientfd, buffer_s, sizeof(buffer_s));
                printf("Received '%s' (%d bytes)\n",buffer_s,n);
                for(cnt=0;cnt<n;cnt++)
                {
                        printf("0x%X ",buffer_s[cnt]);
                }
                printf("\n");
                if (n < 0) {

                        printf("error en read()\n");
                        write(clientfd, "Error", 6);
                        exit(-1);
                }
                if (n == 0)
                {
                        printf("Client gone\n");
                        close(clientfd);
                        break;
                }

                printf("message received: '%s'\n", buffer_s);
        }

The only changes that I've made are the memset (bzero will also do but is deprecated), splitting if(n<=0), some additional printf's for debugging and the '\n' in the last printf

Works as expected (including the bug in the client where you try to write over a closed socket

).

client side

Code:

wim@btd-techweb01:~/progs/lq641325$ ./client
Message to send -> hallo
before: 'hallo
'
after : 'hallo'
Message Ok
Message to send -> do it
before: 'do it
'
after : 'do it'
Message Ok
Message to send -> /h
before: '/h
'
after : '/h'
Help ...
Message Ok
Message to send -> yebo
before: 'yebo
'
after : 'yebo'
Message Ok
Message to send -> /quit
before: '/quit
'
after : '/quit'
closed
error in write()wim@btd-techweb01:~/progs/lq641325$

server side

Code:

wim@btd-techweb01:~/progs/lq641325$ ./server
127.0.0.1:35077 connected
Received 'hallo' (5 bytes)
0x68 0x61 0x6C 0x6C 0x6F
message received: 'hallo'
Received 'do it' (5 bytes)
0x64 0x6F 0x20 0x69 0x74
message received: 'do it'
Received '/h' (2 bytes)
0x2F 0x68
message received: '/h'
Received 'yebo' (4 bytes)
0x79 0x65 0x62 0x6F
message received: 'yebo'
Received '' (0 bytes)

Client gone
wim@btd-techweb01:~/progs/lq641325$

Conclusion: learn to debug (in this case simple printf's do the trick) so you can see what is actually transmitted and received.

lrios · 05-13-2008, 08:42 AM

Oohhhh men thx a lot

. Now my server receive the message

So many problems for a simple "\n"

paulsm4 · 05-14-2008, 05:55 PM

lrios -

Hopefully you've learned something?