0

I have the following piece of C++ code, wherein I'm trying to execute a routine:

#include <thread>
#include <unistd.h>
#include <sys/wait.h>
#include <memory>

using namespace std;

static void upload(const string url, const string path){ int sync_status;}

int main(){
    bool flag = true;
    for(int worker_id = 0; worker_id < 1; worker_id++){
        int worker_pid = fork();
        if (worker_pid == 0){
            while (true){
                std::unique_ptr<std::thread> uploader(nullptr);
                if (flag){       // This block only occurs once inside while loop
                    flag = false;
                    string path = "l", url = "k";
                    // Valgrind reports the next line
                    uploader = std::make_unique<std::thread>(upload, url, path);
                }
                int child_id = fork();
                if (!child_id){
                    // Do something
                    exit(0);
                }
                waitpid(child_id, NULL, 0);
                if(uploader && uploader->joinable()){
                    uploader->join();
                }
            }
            // Do something
            exit(0);
        }
    }
    return 0;
}

But valgrind seems to always report this code as follows:

==16424== 80 bytes in 1 blocks are definitely lost in loss record 2 of 2
==16424==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16424==    by 0x10AA39: std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> > std::thread::_S_make_state<std::thread::_Invoker<std::tuple<void (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >), std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >(std::thread::_Invoker<std::tuple<void (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >), std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&&) (thread:197)
==16424==    by 0x10A04A: std::thread::thread<void (&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >), std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(void (&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >), std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (thread:126)
==16424==    by 0x109A64: std::_MakeUniq<std::thread>::__single_object std::make_unique<std::thread, void (&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >), std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(void (&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >), std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (unique_ptr.h:825)
==16424==    by 0x109514: main (main.cpp:21)

what is the reason behind this leak? I thought that this was due to the exit() call but even joining the unique_ptr before the call does not help.

  • That program is very hard to follow, as you use `std::thread`, POSIX threads, and forks new processes (which you seem to exit immediately anyway so I don't see the point of it). You also seem to not really use the `upload` thread as a thread, as you start it and then wait for it to finish before doing anything else, so it's not really running in parallel and you could just called the function directly instead. I also don't see the need to have the `std::thread` wrapped in a smart pointer. – Some programmer dude Jun 01 '18 at 08:01
  • @Someprogrammerdude There are two calls to `fork` there (which makes it even more confusing). – melpomene Jun 01 '18 at 08:02
  • This is basically a small gist of a larger structure with the unrelated routines removed. So it is a minimal example. Basically the idea is that the child subroutines should occur independently from the main worker. – Shikhar Jaiswal Jun 01 '18 at 08:03
  • 1
    This is not minimal. There are several lines that don't do anything or are nonsensical. – melpomene Jun 01 '18 at 08:04
  • @melpomene Please let me know where I can improve the code, I've tried my best, sorry if it doesn't make sense :( – Shikhar Jaiswal Jun 01 '18 at 08:09
  • Go over every line. Check if you can you remove it and still reproduce the problem. If so, delete the line. For example, do you really need `#include `? – melpomene Jun 01 '18 at 08:10
  • add `-ggdb3` on the compilation commands to know the exact line, you are getting memory leak – HarshGiri Jun 01 '18 at 08:10
  • @HarshGiri We already know the line. – melpomene Jun 01 '18 at 08:11
  • 3
    Besides learning how learn how to create a [Minimal, Complete, and Verifiable Example](http://stackoverflow.com/help/mcve), I also recommend you [learn how to debug your code](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). But most importantly, [find a simpler problem](https://ericlippert.com/2014/03/21/find-a-simpler-problem/). By removing anything that's unrelated to your problem you can narrow it down more and more until the only code left is the simplest code possible to recreate the problem, and you should hopefully know exactly when and where it happens, and also why. – Some programmer dude Jun 01 '18 at 08:11
  • @melpomene I have edited out as much as I could without changing the valgrind output. Thanks for your suggestions! – Shikhar Jaiswal Jun 01 '18 at 08:44
  • from the pid in the valgrind log is the leak comming from the child process or the parent ? – Tyker Jun 01 '18 at 08:47
  • Also, the line numbers in the Valgrind output you show are no longer valid. Can you please update with the output using the code you show, and add a comment on the line where it happens? – Some programmer dude Jun 01 '18 at 08:48
  • I have mentioned where the valgrind reports the error to originate from – Shikhar Jaiswal Jun 01 '18 at 08:54
  • @Tyker The mentioned reduced log is all I get by running the command: valgrind --leak-check=full --show-leak-kinds=all ./main – Shikhar Jaiswal Jun 01 '18 at 08:55
  • 2
    Note that you can reproduce the same with this code: https://pastebin.com/QpbF5aj2 – nos Jun 01 '18 at 09:07
  • 1
    aside: mixing forking and using threads is something you really don't want to do. http://www.linuxprogrammingblog.com/threads-and-fork-think-twice-before-using-them – UKMonkey Jun 01 '18 at 09:14

1 Answers1

3

You can find this sentence in [basic.start.main]:

Terminating the program without leaving the current block (e.g., by calling the function std​::​exit(int) ([support.start.term])) does not destroy any objects with automatic storage duration ([class.dtor]).

So your forked copy of the unique_ptr will never be destroyed. That is the leak valgrind reports.

Not that trying to destroy two copies of a unique_ptr seems like a great idea either. So there is probably a deeper flaw in the design of this example.

Bo Persson
  • 90,663
  • 31
  • 146
  • 203
  • Please pardon my relative inexperience with C++, and thanks for this amazing deduction. I had read at places that inside main, exit() and return() pretty much offer the same functionality, so there certainly is a lot to learn for me. What should be the best course of action according to you, keeping the changes minimal? – Shikhar Jaiswal Jun 01 '18 at 10:42
  • 1
    I don't know really. The `fork()` call creates a separate process [with only a single thread](https://stackoverflow.com/questions/1073954/fork-and-existing-threads). So what happens to the cloned `std::thread`s now referring to threads in the original program? Nothing good probably. – Bo Persson Jun 01 '18 at 11:00
  • Is there a way to access a parent fork's variables from a child fork in UNIX? – Shikhar Jaiswal Jun 04 '18 at 08:48
  • 1
    No easy way, at least. The difference between a thread and a "fork" is that all threads in a program share the same memory, while `fork()` creates a separate process ("a new program") with its own separate memory. – Bo Persson Jun 04 '18 at 10:13