arun11299 / cpp-subprocess

Subprocessing with modern C++
Other
456 stars 90 forks source link

Segfault when creating and closing lots of subprocesses #52

Open mplough opened 4 years ago

mplough commented 4 years ago

This behavior is seen in 126ee29.

Steps to reproduce

Compile and run the following program on a non-Windows system:

#include <iostream>
#include "subprocess.hpp"
using namespace subprocess;

int main(int argc, char **argv) {
    for (int i=0; i<1000; i++) {
        std::cout << i << std::endl;
        auto p = Popen({"grep", "f"}, output{PIPE}, input{PIPE});
        auto msg = "one\ntwo\nthree\nfour\nfive\n";
        p.send(msg, strlen(msg));
        auto res = p.communicate();
    }
}

Expected behavior

The program runs for 1000 iterations and exits.

Actual behavior

The program runs for some iterations and segfaults. The number of iterations changes as ulimit -n is changed.

Likely cause

Running lsof at the end shows that only six file descriptors are open. However, if file descriptor is made accessible via a FILE * via fdopen, it's necessary to close it with fclose. The orphaned file descriptor won't show up in lsof but it will count against the limit.

arun11299 commented 4 years ago

Will take a look at this. Thanks for reporting.

vvviperrr commented 4 years ago

seems like everything works normal. g++ (Arch Linux 9.2.1+20200130-2) 9.2.1 20200130

nils1603 commented 1 year ago

I think the issue still exists. My program segfaults as well.

The error shows up, when a new file_ptr is supposed to be opened here: https://github.com/arun11299/cpp-subprocess/blob/1392c47cbbf519bd02ddafb5871526c16b7f2dd1/subprocess.hpp#L1632

I have adjusted this line, to check the error and it shows that too many files are opened and segfaults in the following read_atmost_n line, since the file could not be opened

auto file_ptr = fdopen(err_rd_pipe, "r");

if (file_ptr == NULL)
{
    printf("Oh dear, something went wrong with read()! %s\n", strerror(errno));
}

The file descriptors do not show up in lsof, as observed above.

I have noticed a difference between the usage of the reset function for the std::shared_ptr<FILE> https://github.com/arun11299/cpp-subprocess/blob/1392c47cbbf519bd02ddafb5871526c16b7f2dd1/subprocess.hpp#L1130

I think in the case below, the filepointer is actually not properly closed and the deleted file descriptor still counts against the limit. https://github.com/arun11299/cpp-subprocess/blob/1392c47cbbf519bd02ddafb5871526c16b7f2dd1/subprocess.hpp#L1888

I am not sure if this needs to be adjusted and if I understand the std API correctly.