MrPeanutButta / socket2me

easier TCP sockets (static) library for Linux
8 stars 5 forks source link

Memory leak on server side after client process is killed #10

Open MrPeanutButta opened 7 years ago

MrPeanutButta commented 7 years ago

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

MrPeanutButta commented 7 years ago

@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

WithFoxSquirrel commented 7 years ago

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

MrPeanutButta commented 7 years ago

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.

WithFoxSquirrel commented 7 years ago
       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.
MrPeanutButta commented 7 years ago

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.

WithFoxSquirrel commented 7 years ago

Any temporary solution will be appreciated ! thanks !

MrPeanutButta commented 7 years ago

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.

MrPeanutButta commented 7 years ago

@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.