Search code examples
c++valgrindifstreamofstream

How to correctly use ifstreams in c++, with subprocess and avoid leaks?


I'm trying to create a pizzeria simulation to learn more about sub-processes and threads. I want to avoid memory leaks at maximum.

For now when I create only one kitchen there is no errors but when I create 2 or more I got some leaks with ifstreams and ofstreams when destroying.

There is the main():

int main(int argc __attribute__((unused)), char const *argv[] __attribute__((unused)))
{
    Kitchen k(0, 2, 30);
    Kitchen k2(0, 2, 30);

    return 0;
}

the Kitchen.hpp:

class Kitchen {
    private:

        bool                                _IsFull = false;
        int                                 _Pid = 0;

        int                                 _CookTime;
        int                                 _MaxCooks;
        long                                _RefreshDelay;
        double                              _MaxTime = 5;
        unsigned long int                   _QueueSize;

        std::ofstream                       _Opipe;
        std::ifstream                       _Ipipe;
        unsigned int                        _Door = 0;
        std::string                         _FallIn = "";
        std::string                         _FallOut = "";

        void CooksAwakening (void);
        void CleaningTime (void);
        void OpenPipe (void);
        void Quit (void);
        void Run (void);

    public:
        explicit Kitchen (int CookingTime, int MaxCooks, long RefreshDelay);
        virtual ~Kitchen ();

        bool IsFull () const { return _IsFull; }
        int GetPid () const { return _Pid; }
    };

And the Kitchen.cpp:

int Knum = 0;

Kitchen::Kitchen(int CookingTime, int MaxCooks, long RefreshDelay)
: _CookTime(CookingTime), _MaxCooks(MaxCooks), _RefreshDelay(RefreshDelay), _QueueSize(_MaxCooks * 2)
{
    OpenPipe();
    _Pid = fork();
    if (_Pid == 0) {
        _Opipe.open(_FallIn.c_str(), std::ostream::out);
        _Ipipe.open(_FallOut.c_str(), std::istream::in);
        Run();
    } else {
        _Ipipe.open(_FallIn.c_str(), std::istream::in);   // Valgrind point this line
        _Opipe.open(_FallOut.c_str(), std::ostream::out); // Valgrind point this line too
    }
}

Kitchen::~Kitchen()
{
    if (_Pid == 0) {
    } else {
        _Opipe << "QUIT" << std::endl;
        _Ipipe.close();
        _Opipe.close();
        unlink(_FallIn.c_str());
        unlink(_FallOut.c_str());
    }
}

void Kitchen::Quit(void)
{
    CleaningTime();
    _Ipipe.close();
    _Opipe.close();
    exit(0);
}

void Kitchen::CleaningTime(void)
{
    while (!_Cooks.empty()) {
        _Cooks.pop_back();
    }
    while (!_PizzaQueue.empty()) {
        _PizzaQueue.pop_back();
    }
    _Cooks.shrink_to_fit();
    _PizzaQueue.shrink_to_fit();
}

void Kitchen::OpenPipe(void)
{
    std::cout << "Kit Open Pipes\t" << getpid() << '\n';
    std::ostringstream       oss1;
    oss1 << "/tmp/kint" << Knum;
    _FallIn = oss1.str();

    std::ostringstream      oss2;
    oss2 << "/tmp/kout" << Knum;
    _FallOut = oss2.str();

    _Door = Knum;
    ++Knum;

    if (mkfifo(_FallOut.c_str(), 0666) != 0) {
        perror ("mkfifo1");
        exit(84);
    }
    if (mkfifo(_FallIn.c_str(), 0666) != 0) {
        perror("mkfifo2");
        exit(84);
    }
}

void    Kitchen::Run(void)
{
    std::string cmd;

    while (_Ipipe >> cmd)
    {
        if (cmd == "QUIT") {
            Quit();
        }
    }
    Quit();
}

This is the valgrind result:

total heap usage: 36 allocs, 32 frees, 109,712 bytes allocated
==20890== 
==20890== 552 bytes in 1 blocks are still reachable in loss record 1 of 4
==20890==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==20890==    by 0x4C1536E: __fopen_internal (in /usr/lib64/libc-2.28.so)
==20890==    by 0x4925AA3: std::__basic_file<char>::open(char const*, std::_Ios_Openmode, int) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x496789D: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x4967A73: std::basic_ifstream<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x4052F2: WorkSpace::Kitchen::Kitchen(int, int, long) (Kitchen.cpp:36)
==20890==    by 0x40250C: main (main.cpp:46)
==20890== 
==20890== 552 bytes in 1 blocks are still reachable in loss record 2 of 4
==20890==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==20890==    by 0x4C1536E: __fopen_internal (in /usr/lib64/libc-2.28.so)
==20890==    by 0x4925AA3: std::__basic_file<char>::open(char const*, std::_Ios_Openmode, int) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x496789D: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x4967AC3: std::basic_ofstream<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x40531C: WorkSpace::Kitchen::Kitchen(int, int, long) (Kitchen.cpp:37)
==20890==    by 0x40250C: main (main.cpp:46)
==20890== 
==20890== 8,192 bytes in 1 blocks are still reachable in loss record 3 of 4
==20890==    at 0x4839593: operator new[](unsigned long) (vg_replace_malloc.c:433)
==20890==    by 0x496358F: std::basic_filebuf<char, std::char_traits<char> >::_M_allocate_internal_buffer() (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x49678B5: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x4967A73: std::basic_ifstream<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x4052F2: WorkSpace::Kitchen::Kitchen(int, int, long) (Kitchen.cpp:36)
==20890==    by 0x40250C: main (main.cpp:46)
==20890== 
==20890== 8,192 bytes in 1 blocks are still reachable in loss record 4 of 4
==20890==    at 0x4839593: operator new[](unsigned long) (vg_replace_malloc.c:433)
==20890==    by 0x496358F: std::basic_filebuf<char, std::char_traits<char> >::_M_allocate_internal_buffer() (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x49678B5: std::basic_filebuf<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x4967AC3: std::basic_ofstream<char, std::char_traits<char> >::open(char const*, std::_Ios_Openmode) (in /usr/lib64/libstdc++.so.6.0.25)
==20890==    by 0x40531C: WorkSpace::Kitchen::Kitchen(int, int, long) (Kitchen.cpp:37)
==20890==    by 0x40250C: main (main.cpp:46)
==20890== 
==20890== LEAK SUMMARY:
==20890==    definitely lost: 0 bytes in 0 blocks
==20890==    indirectly lost: 0 bytes in 0 blocks
==20890==      possibly lost: 0 bytes in 0 blocks
==20890==    still reachable: 17,488 bytes in 4 blocks
==20890==         suppressed: 0 bytes in 0 block

I compile with gcc and these flags: -Wall -Wextra -Weffc++

And the valgrind command is as follow: valgrind --leak-check=full --show-leak-kinds=all ./plazza


Solution

  • The shown code executes the following in a child process:

    void Kitchen::Quit(void)
    {
        CleaningTime();
        _Ipipe.close();
        _Opipe.close();
        exit(0);
    }
    

    Even though the open streams are manually closed, those object still exists at this point, and exit(0) terminates the process immediately.

    Despite the actual files getting closed, the stream objects still exist, and still have some memory allocated to them, for their internal stream buffers. That memory only gets released when these objects get properly destroyed, which doesn't happen via exit(0).

    exit(0) is a standard C library function that doesn't know anything about any C++ objects. It just nukes the process from high orbit.

    valgrind is detecting that the subprocess terminated without fully releasing all the memory it allocated, and reports that.

    To "correctly use ifstreams in C++ with subprocesses and avoid leaks", subprocesses must terminate the same way that the main process terminates: by returning from main(). This will obviously destroy all objects in automatic and static scope, in the subprocess, before terminating it.

    This obviously poses several difficult problems to solve, in terms of how the program logically works. One common, brute force approach that will often work is throwing an exception instead of exit()ing, which gets caught in main(). This only works, of course, with code that's structured to behave correctly when exceptions get thrown.