0

I'm writing some TCP server code on linux, it's being cross-compiled for a Pi with buildroot tool chain. Basically I'm accepting a socket then creating a thread to listen for data and process it. For testing I'm using telnet as a client. Occasionally when the socket closes the program crashes during thread clean-up and I can't figure out why. My best guess is file-descriptor weirdness but that's mostly a guess at this point.

Here's the code:

void NetworkControl::startListening()
{
    mListenThread = std::thread(listener);
    mListenThread.detach();
}

void NetworkControl::listener()
{
    int sockfd, newsockfd, clilen;
    struct sockaddr_in serv_addr, cli_addr;

    int portno = 612;

    std::thread mWorkThread;

    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    if (sockfd < 0) 
       {} //print error

    bzero((char *) &serv_addr, sizeof(serv_addr));

    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = INADDR_ANY;
    serv_addr.sin_port = htons(portno);
    if (bind(sockfd, (struct sockaddr *) &serv_addr,sizeof(serv_addr)) < 0) 
    { }//print error

    listen(sockfd,5);

    clilen = sizeof(cli_addr);

    while (1) { 
        newsockfd = accept(sockfd, 
            (struct sockaddr *) &cli_addr, (socklen_t*) &clilen);
        if (newsockfd < 0) {
            //print error
        }else{
            mWorkThread = std::thread(worker,newsockfd);
            mWorkThread.detach();
        }
    }
}

void NetworkControl::worker(int sock)
{
    uint32_t n;

    char *buf = new char(200);

    while(1){
        n = read(sock,&buf[0],199);;
        if (n <= 0){
            goto exit;
        }
        buf[n] = '\x00';
        printf("%s\n",buf);
    }
    exit:
    free(buf);
    close(sock);
}

Here's the backtrace from the crash:

#0  0x767c4e0c in raise () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#1  0x767c60e4 in abort () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#2  0x767fe2c8 in __libc_message () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#3  0x76804790 in malloc_printerr () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#4  0x76805850 in _int_free () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#5  0x768a3c78 in tcache_thread_freeres () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#6  0x768a3cdc in __libc_thread_freeres () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
#7  0x76f01f3c in start_thread () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libpthread.so.0
#8  0x768683b8 in ?? () from /home/david/Desktop/BR-Pi2/output/host/arm-buildroot-linux-gnueabihf/sysroot/lib/libc.so.6
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

The console output:

free() invalid next size (fast)

If anyone has any ideas I'd appreciate it!

David
  • 93
  • 4
  • I assume that `NetworkControl::listener` is a `static` member function? Can you please try to create an [mcve] to show us? – Some programmer dude Nov 22 '18 at 07:34
  • As for the message you get, it's typical when you write out of bounds of allocated memory. Use e.g. [Valgrind](http://valgrind.org/) to help you find out more. – Some programmer dude Nov 22 '18 at 07:35
  • 3
    A hint: `new char(200)` allocates a ***single*** `char`, and initializes it to the value `200`. – Some programmer dude Nov 22 '18 at 07:36
  • 2
    Furthermore: Always match `new` with `delete`, and `new[]` with `delete[]`. Perhaps you should take a few steps back, [find a good book or two](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282) and read more about dynamic memory allocation (and quite a few things more IMO)? Or better yet, use `std::string` for all strings. – Some programmer dude Nov 22 '18 at 07:37
  • Lastly, never use `goto` and labels. If you need to break out of a loop use `break`. – Some programmer dude Nov 22 '18 at 07:37
  • @Someprogrammerdude What's the problem with `goto`? – curiousguy Nov 22 '18 at 08:28
  • @curiousguy You mean besides the usual "goto considered harmful"? [This old answer](https://stackoverflow.com/a/52307/440558) have a few very good points. I'm sure that more can be made. – Some programmer dude Nov 22 '18 at 08:37
  • @Someprogrammerdude the new char(200) is exactly it. Should've known it be something simple, and that's only new statement in my code as it was obviously a memory problem somewhere. – David Nov 22 '18 at 14:41

2 Answers2

1

As already told be others the crash is caused by pairing new() with free() while you actually needed new[]/delete[].

However, using a std::vector, you don't even have to do the memory allocation yourself. (example code not compiled/tested)

void NetworkControl::worker(int sock)
{
    uint32_t n;

    std::vector<char> buf(200);

    while(1){
        n = read(sock, buf.data(), 199);
        if (n <= 0){
            break;
        }
        buf[n] = '\x00';
        printf("%s\n",buf.data());
    }
    close(sock);
}

For buf you could also use a C-style array or std::array<char>, but then you have to make sure you don't get a stackoverflow.

stefaanv
  • 14,072
  • 2
  • 31
  • 53
1

The free() function frees the memory space pointed to by ptr, which must have been returned by a previous call to malloc(), calloc() or realloc(). Other‐ wise, or if free(ptr) has already been called before, undefined behavior occurs. If ptr is NULL, no operation is performed.

but in your code context:

char *buf = new char(200);

the pointer is getted by new function, it's a C++ function, That's why your program crashed.

L.Hao
  • 27
  • 1