Search code examples
cvalgrind

Child process memory free problem with execvp


The following code is from the book "Operating Systems: Three Easy Pieces". The code is confusing me. I know execvp never returns when it works well.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
#include <assert.h>
#include <sys/wait.h>

int
main(int argc, char *argv[])
{
    int rc = fork();
    if (rc < 0) {
        // fork failed; exit
        fprintf(stderr, "fork failed\n");
        exit(1);
    } else if (rc == 0) {
        // child: redirect standard output to a file
        close(STDOUT_FILENO); 
        open("./p4.output", O_CREAT|O_WRONLY|O_TRUNC, S_IRWXU);

        // now exec "wc"...
        char *myargs[3];
        myargs[0] = strdup("wc");   // program: "wc" (word count)
        myargs[1] = strdup("p4.c"); // argument: file to count
        myargs[2] = NULL;           // marks end of array
        execvp(myargs[0], myargs);  // runs word count
    } else {
        // parent goes down this path (original process)
        int wc = wait(NULL);
    assert(wc >= 0);
    }
    return 0;
}

I use Valgrind to check for memory leaks. The above code does no memory leak. When I delete the execvp line, it will detect definitely lost: 8 bytes in 2 blocks. Why is this?

Valgrind command:

valgrind --leak-check=full ./a.out

when I use command valgrind --trace-children=yes --leak-check=full ./p4

==15091== Memcheck, a memory error detector
==15091== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==15091== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==15091== Command: ./p4
==15091==
==15092== Memcheck, a memory error detector
==15092== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==15092== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==15092== Command: /usr/bin/wc p4.c
==15092==
==15092==
==15092== HEAP SUMMARY:
==15092==     in use at exit: 0 bytes in 0 blocks
==15092==   total heap usage: 36 allocs, 36 frees, 8,809 bytes allocated
==15092==
==15092== All heap blocks were freed -- no leaks are possible
==15092==
==15092== For counts of detected and suppressed errors, rerun with: -v
==15092== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==15091==
==15091== HEAP SUMMARY:
==15091==     in use at exit: 0 bytes in 0 blocks
==15091==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==15091==
==15091== All heap blocks were freed -- no leaks are possible
==15091==
==15091== For counts of detected and suppressed errors, rerun with: -v
==15091== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
[root cpu-api]#

no matter how many bytes I malloc,the heap summary always says: ==15092== total heap usage: 36 allocs, 36 frees, 8,809 bytes allocated


Solution

  • What's "definitely lost" memory?

    First of all, let's discuss what Valgrind reports as "definitely lost": Valgrind will report allocated memory as "definitely lost" if all references to the allocated memory are lost before the termination of the program. In other words, if your program reaches a state in which there is allocated memory that cannot be freed because no valid pointers to it exists, this will count as "definitely lost".

    This means that a program like this:

    int main(void) {
        char *buf = malloc(10);
        // ...
        exit(0);
    }
    

    will cause no error from Valgrind, while a program like this:

    void func(void) {
        char *buf = malloc(10);
        // ...
    } // memory is definitely lost here
    
    int main(void) {
        func();
        exit(0);
    }
    

    will cause a "definitely lost" error.

    Why is the first version ok for Valgrind? It's because memory is always freed by the system on program exit. If you keep using an allocated chunk of memory until the end of your program, there really is no need to explicitly call free() on it, and it can be considered as just a waste of time. For this reason, if you don't free some allocated block while still holding a reference to it, Valgrind assumes that you did so to avoid an "useless" free() because you are smart and know that the OS is going to take care of it anyway.

    If however you forget to free() something, and lose every reference to it, then Valgrind warns you, because you should have free()d the memory. If you don't, and the program keeps running, then the same thing happens each time the buggy block is entered, and you end up wasting memory. This is what is called a "memory leak". A very simple example is the following:

    void func(void) {
        char *buf = malloc(10);
        // ...
    } // memory is definitely lost here
    
    int main(void) {
        while (1) {
            func();
        }
        exit(0);
    }
    

    This program will make your machine run out of memory and could ultimately end up killing or freezing your system (warning: do not test this if you don't want to risk freezing your PC). If you instead correctly call free(buf) before the end of func, then the program keeps running indefinitely without a problem.

    What happens in your program

    Now let's see where you are allocating memory and where the variables holding the references are declared. The only part of the program that is allocating memory is inside the if (rc == 0) block, through strdup, here:

    char *myargs[3];
    myargs[0] = strdup("wc");   // program: "wc" (word count)
    myargs[1] = strdup("p4.c"); // argument: file to count
    

    The two calls to strdup() duplicate the string and allocate new memory to do so. Then, you save a reference to the newly allocated memory in the myargs array, which is declared inside the if block. If your program exits the block without freeing the allocated memory, then those references will be lost, and there will be no way for your program to free the memory.

    With execvp(): your child process is replaced by the new process (wc p4.c), and the memory space of the parent process is thrown away by the operating system (for Valgrind, this is exactly the same as program termination). This memory is not counted as lost by Valgrind, because references to the allocated memory are still present when execvp() is called. NOTE: this is not becaue you pass the pointers to allocated memory to execvp(), it's because the original program effectively terminates and memory is retained by the OS.

    Without execvp(): your child process continues execution, and right after it exits the code block where myargs is defined, it loses any reference to the allocated memory (since myargs[0] and myargs[1] were the only references). Valgrind then correctly reports this as "definitely lost", 8 bytes (3 for "wc" and 5 for "p4.c") in 2 blocks (2 allocations). The same thing happens if the call to execvp() fails for whatever reason.

    Additional considerations

    To be fair, there is no real need to call strdup() in the program you show. It's not like those strings need to be copied because they are used somewhere else (or anything like that). The code could have simply been:

    myargs[0] = "wc";   // program: "wc" (word count)
    myargs[1] = "p4.c"; // argument: file to count
    

    In any case, a good practice when using the exec*() family of functions, is to put an exit() directly after it, to ensure the program doesn't keep running in case the exec*() fails. Something like this:

    execvp(myargs[0], myargs); 
    perror("execvp failed");
    exit(1);