Hello,

I've written a client / server application that is multithreaded with
pthreads - I now I'm second guessing my decision.

Basically the server waits for connections,
starts a new pthread when connection is recieved,
receieves data,
does some work,
and then sends back a response.

I'm having some memory problems  -  probably because this is my first
threaded application and I don't know the ins and outs yet.  For this type
of application should I have just forked a new process to handle each client
request?

Also - I'm having a problem with my recvall function.  Sometimes ther recv
call just sits there ... and doesn't do anything.  I'm assuming the client
disconnected before is sent any data?  The problem is that b/c of my memory
problems I have at the beggining of the thread a mutex lock and at the end a
mutex unlock :-)  So, when recv sits and wiats forever connections are
piling up...

I'm kinda in a bind because when I was testing I didn't have memory problems
or recv problems ... and now the software is live doing about 200,000
connections per day - and I have to sit and watch it like 24/7 to restart on
the recv errors :-(

Here is my recv all function:

[ Post a follow-up to this message ]

"Matt Gostick" <matt@crazylogic.net> wrote in message
news:L3iwd.86415$vO1.503995@nnrp1.uunet.ca...
> Hello,
>
> I've written a client / server application that is multithreaded with
> pthreads - I now I'm second guessing my decision.
>
> Basically the server waits for connections,
> starts a new pthread when connection is recieved,
>    receieves data,
>    does some work,
>    and then sends back a response.
>
> I'm having some memory problems  -  probably because this is my first
> threaded application and I don't know the ins and outs yet.  For this type
> of application should I have just forked a new process to handle each
> client request?
>
> Also - I'm having a problem with my recvall function.  Sometimes ther recv
> call just sits there ... and doesn't do anything.  I'm assuming the client
> disconnected before is sent any data?  The problem is that b/c of my
> memory problems I have at the beggining of the thread a mutex lock and at
> the end a mutex unlock :-)  So, when recv sits and wiats forever
> connections are piling up...
>
> I'm kinda in a bind because when I was testing I didn't have memory
> problems or recv problems ... and now the software is live doing about
> 200,000 connections per day - and I have to sit and watch it like 24/7 to
> restart on the recv errors :-(
>
> Here is my recv all function:
>
>
>

damn copy and paste - technology hates me;

int recvall (int new_fd, char **request) {
char *buffer;
char *storage;
int bytes_recv = 1;
int total_recv = 0;
int i,j;

printf("%s - recv: start\n", timestamp());
fflush(stdout);

buffer = (char *) malloc((MAXBYTES+1) * sizeof(char));
strcpy(buffer, "");

/* I use a storage variable b/c remalloc is crazy and makes a new pointer
sometimes */
storage = (char *) malloc(2 * sizeof(char));
strcpy(storage, "");

while (bytes_recv > 0) {

bytes_recv = recv(new_fd, buffer, MAXBYTES, 0);

if (bytes_recv > 0) {

/* finish the string */

if (buffer[bytes_recv-1] == '\n') {
bytes_recv--;
buffer[bytes_recv-1] = '\0';
} else {
buffer[bytes_recv] = '\0';
}

total_recv += bytes_recv;

/* add to the input */
storage = (char *) realloc(storage, (strlen(storage)+strlen(buffer)+1)
* sizeof(char));
strcat(storage, buffer);

/* finish the string - for now */
storage[total_recv] = '\0';

printf("%s - recv: %3d %3d %s\n", timestamp(), bytes_recv, total_recv,
buffer);
fflush(stdout);

}

if (strstr(storage,"EOF")) {
break;
}

/* sometimes nothing is sent... */
if (bytes_recv == 0) {
bytes_recv = -1;
break;
}

}

/* get rid of the last \n */
storage[strlen(storage)-1] = '\0';

/* copy the output into original string */
*request = (char *)malloc((strlen(storage)+1) * sizeof(char));

//printf("%x\n", *request);

strcpy(*request, storage);

free(storage);
free(buffer);

printf("%s - recv: stop\n", timestamp());
fflush(stdout);

/* return total recv or -1 for failure */
return total_recv;
}

[ Post a follow-up to this message ]

In article <I6iwd.86416$vO1.503871@nnrp1.uunet.ca>,
"Matt Gostick" <matt@crazylogic.net> wrote:

> "Matt Gostick" <matt@crazylogic.net> wrote in message
> news:L3iwd.86415$vO1.503995@nnrp1.uunet.ca... 

It should work either way.  But if there's no interaction between the
handlers for each connection, or between these and the master thread,
then processes may be easier since you don't have to worry as much about
mutual exclusion.

What kind of "memory problems" are you having?
[vbcol=seagreen] 
>
> damn copy and paste - technology hates me;
>
> int recvall (int new_fd, char **request) {
>   char *buffer;
>   char *storage;
>   int bytes_recv = 1;
>   int total_recv = 0;
>   int i,j;
>
>   printf("%s - recv: start\n", timestamp());
>   fflush(stdout);
>
>   buffer = (char *) malloc((MAXBYTES+1) * sizeof(char));

Don't cast the result of malloc().  It prevents the compiler from
warning you if you didn't prototype malloc() properly.

>   strcpy(buffer, "");
>
>   /* I use a storage variable b/c remalloc is crazy and makes a new pointe
r
> sometimes */

That's what it's supposed to do, what's so crazy about that?

>   storage = (char *) malloc(2 * sizeof(char));

Again, don't cast malloc().

>   strcpy(storage, "");
>
>   while (bytes_recv > 0) {
>
>     bytes_recv = recv(new_fd, buffer, MAXBYTES, 0);
>
>     if (bytes_recv > 0) {
>
>       /* finish the string */
>
>       if (buffer[bytes_recv-1] == '\n') {
>         bytes_recv--;
>         buffer[bytes_recv-1] = '\0';

That should be buffer[bytes_recv].  Instead of overwriting the newline,
you're overwriting the last character of the line.

But why do you only remove newlines at the end of the buffer?  There's
no reason to expect the buffers to be aligned on line boundaries.

>       } else {
>         buffer[bytes_recv] = '\0';
>       }
>
>       total_recv += bytes_recv;
>
>       /* add to the input */
>       storage = (char *) realloc(storage, (strlen(storage)+strlen(buffer)+
1)
> * sizeof(char));
>       strcat(storage, buffer);
>
>       /* finish the string - for now */
>       storage[total_recv] = '\0';

Strcat() does this for you.

>
>       printf("%s - recv: %3d %3d %s\n", timestamp(), bytes_recv, total_rec
v,
> buffer);
>       fflush(stdout);
>
>     }
>
>     if (strstr(storage,"EOF")) {
>       break;
>     }
>
>     /* sometimes nothing is sent... */
>     if (bytes_recv == 0) {
>       bytes_recv = -1;
>       break;
>     }
>
>   }
>
>   /* get rid of the last \n */
>   storage[strlen(storage)-1] = '\0';

You didn't copy the \n's into storage in the first place.

>
>   /* copy the output into original string */
>   *request = (char *)malloc((strlen(storage)+1) * sizeof(char));
>
>   //printf("%x\n", *request);
>
>   strcpy(*request, storage);
>
>   free(storage);
>   free(buffer);
>
>   printf("%s - recv: stop\n", timestamp());
>   fflush(stdout);
>
>   /* return total recv or -1 for failure */
>   return total_recv;
> }

--
Barry Margolin, barmar@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***

[ Post a follow-up to this message ]

> What kind of "memory problems" are you having?

I'm not exactly sure where the problem in my code is .. 1. I haven't touched
C in years and 2. This is my first mutlithreaded application.  I think I'm
going to switch to fork() today instead so I don't have to worry about it
anymore...

Regardless, the problem I'm having is not a memory leak.  It seems that the
request variable filled by recvall function gets overridden if new
connection comes in at the exact instant - and it core dumps.  I should
note:  request is not defined globally.  sendrecv is in sendrecv.o (compiled
with -lpthread).  And that's about it... the rest is what you see here.  I
thought the only problems I would have is if I defined any static sized
arrays...  but I guess not.
 

recv() is definately my more major problem right now...
[vbcol=seagreen] 
>
> Don't cast the result of malloc().  It prevents the compiler from
> warning you if you didn't prototype malloc() properly.

okay.
 
>
> That's what it's supposed to do, what's so crazy about that?

Well it's not that remalloc is crazy - it's just that I don't want this.  I
passed this function a pointer (**request) .. and remalloc was creating a
new pointer sometimes to make room .. but the calling function didn't have
the updated value.  I don't exactly remember - and it makes sense now to me
that it should have worked anyway - this was pretty much the first chunk of
I've written for the program... and I was rusty.  I will fix this later as I
don't think it's the source of my problems right now...

> 
>
> Again, don't cast malloc().
> 
>
> That should be buffer[bytes_recv].  Instead of overwriting the newline
,
> you're overwriting the last character of the line.

I'm removeing ^M\n here ... not just \n.  It's a visual basic program thats
connecting to this...

> But why do you only remove newlines at the end of the buffer?  There's
> no reason to expect the buffers to be aligned on line boundaries.

Well, the VB app has no new lines in the input really.... except at the end
of the input... EOF^M\n will be the last five characters.

> 
>
> Strcat() does this for you.

okay...

> 
>
> You didn't copy the \n's into storage in the first place.

Correct - must just be a remnant ...  but it would only get rif of the F in
EOF... so it doesn't make a difference as I don't use that portion of the
input anyway.
[vbcol=seagreen] 

[ Post a follow-up to this message ]

In article <iFDwd.86464$vO1.504634@nnrp1.uunet.ca>,
"Matt Gostick" <matt@crazylogic.net> wrote:
 
>
> recv() is definately my more major problem right now...

Recv() will wait forever if the client doesn't call close() or
shutdown() after sending the command.  A blocking recv() only returns
when there's data available, EOF has been received, or a signal
interrupts it.

--
Barry Margolin, barmar@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***

[ Post a follow-up to this message ]

I was able to set a timeout and solve my problem calling this right before I
did the recv.

/*
*
* http://www.rtems.com/ml/rtems-users...r/msg00255.html
*
*/

int recvall_timeout (int new_fd, int milliseconds) {
struct timeval tv;

tv.tv_sec = milliseconds / 1000 ;
tv.tv_usec = ( milliseconds % 1000) * 1000  ;

return setsockopt(new_fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof
tv);
}


"Barry Margolin" <barmar@alum.mit.edu> wrote in message
news:barmar-4DC0E4.20430017122004@comcast.dca.giganews.com...
> In article <iFDwd.86464$vO1.504634@nnrp1.uunet.ca>,
> "Matt Gostick" <matt@crazylogic.net> wrote:
> 
>
> Recv() will wait forever if the client doesn't call close() or
> shutdown() after sending the command.  A blocking recv() only returns
> when there's data available, EOF has been received, or a signal
> interrupts it.
>
> --
> Barry Margolin, barmar@alum.mit.edu
> Arlington, MA
> *** PLEASE post questions in newsgroups, not directly to me ***

[ Post a follow-up to this message ]