MrPeanutButta/socket2me

Memory leak on server side after client process is killed

Opened this issue · 8 comments

Tracking a potential bug regarding the server when the client process is terminated.

@ahebert , sorry for late reply,

1、i start a server like following:

` unique_ptr s(new server("vcm_plugin_auth", auth::MD5));

s->set_read_callback(server_receive);
s->listen("0.0.0.0", port);

while(1)
{
    this_thread::sleep_for(chrono::seconds(50));
}`

2、and then start a client without addfailover

` unique_ptr c(new client("vcm_plugin_auth", auth::MD5));

static int i = 0;

 while(!c->authenticate(ip_addr, port))
{
    cout<< "authenticate fail, please check the server ip and port is correct " << endl;

    client_s(to_string(++i));
    i = i < 10 ? i : 3;

    this_thread::sleep_for(chrono::seconds(kHeartBeatInterval));
};

do {

    while (c->connected()) 
    {

        auto uuid(c->getUUid());
		
		//do something
		
        this_thread::sleep_for(chrono::seconds(kHeartBeatInterval));
    }

    client_s(to_string(++i));
    i = i < 10 ? i : 3;

    this_thread::sleep_for(chrono::seconds(kHeartBeatInterval));

    cout << "attempting failover" << endl;

} while (c->failover());`

3、when i kill the client process , the server memory increasing fast ,
maybe i have wrong way to use your software, could you review my code,
and give me some advices? thanks a lot

hi,ahebert, have you reproduced this phenomenon, when terminal the client process ? Could you give me some clues to fix this.

Hey, I didn't have the time to test this. I'm not sure when I can.

valgrind might be able to help with discovering the memory leak. Inspect the listen loops in the server class for any issues.

       valgrind located the possible leak code (server.cpp , function connection_loop, line 204)
       line 204:
       stream += fgetc(ipend.rx);

        // while connected and BGP still running.
        // feof() should be safe for local socket.
        while (!feof(ipend.rx) && !server::kill_) {

            // get chars from stream
            do {
                stream += fgetc(ipend.rx);
            } while (stream.back() != tcp::EOL);
         
          .....

         when i terminal the client process, the cycle still runing and read garbled from file ipend.rx, 
         maybe it's why memory leak.

Good catch. feof should not be used here as it can block.

Also, if client gets SIGINT (ctrl+c), the client never sends EOL and theres no graceful shutdown/break on the server side.

This code needs to be refactored for sure.

Any temporary solution will be appreciated ! thanks !

Unfortunately, I won't be able to get to this until I get some free time. But it appears there is an issue with checking the file descriptor for EOF when the underlying device is a socket.

I recommend trying to read directly from the socket 'int client_socket' and if read returns -1, you can check errorno to determine if the server can recover or if it's fatal.

@songbaoer were you able to apply this workaround successfully?
I'm starting to look into this in my free time. I should have a fix by this weekend.