0

I'm creating a process and reading it's stdout and err via the win32 api. Problem is that after the first successfull read with ReadFile(), I'm always getting the broken pipe error (109) for the next one for the data left in the pipe. (so I only read the content as big as the buffer in my first call) I'm suspecting that it's maybe because I'm reading the pipe synchronlysly but after the child terminated?

void read_pipes(std::string &output, HANDLE read_pipe) {
        assert(read_pipe != nullptr && read_pipe != INVALID_HANDLE_VALUE);
        char buffer[4096];
        DWORD bytes_left_to_read, err_code; int i = 0;
        do {
            memset(&buffer, 0, 4096);
            err_code = 0;
            if (!ReadFile(read_pipe, buffer, 4096 - 1, &bytes_left_to_read, NULL)) {
                err_code = GetLastError();
            }
            assert(err_code != ERROR_IO_PENDING);
            if (err_code != 0 && err_code != ERROR_MORE_DATA) {
                char sys_err_msg[128];
                sys_err_msg[128 - 1] = '\0';
                FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, 0, err_code, MAKELANGID(0x09, 0x01), sys_err_msg, 127, nullptr);
                std::cerr << "Failed to read failed mmdb_importer log, because " << sys_err_msg << ";" << err_code
                        << std::endl;
                break;
            }
            output.append(buffer);
        } while (true);

And part of the code which calls the read (full under http://pastebin.com/vakLULyf )

if (WaitForSingleObject(mddb_importer_info.hProcess, 60000) == WAIT_TIMEOUT) {
        //Kill the importer, probably hangs
        TerminateProcess(mddb_importer_info.hProcess, EXIT_FAILURE);
    }
    GetExitCodeProcess(mddb_importer_info.hProcess, &mddb_importer_return);
    //assert (mddb_importer_return != STILL_ACTIVE);

    switch (mddb_importer_return) {
        case EXIT_SUCCESS:
            file_to_upload.uploaded = true;
            break;
        default:
            assert(file_to_upload.uploaded == false);
        {
            read_pipes(std::ref(file_to_upload.log), mddb_importer_stderr_r);
            std::clog << "MDDB_Importer failed with err: : " << file_to_upload.log << std::endl;
            read_pipes(std::ref(file_to_upload.log), mddb_importer_stdout_r);
            std::clog << "And total size of log " << file_to_upload.log.length() << std::endl;
        }
            break;
    }
Superlokkus
  • 4,731
  • 1
  • 25
  • 57
  • You are not checking the value of `bytes_left_to_read` after `ReadFile()`. Maybe there is something unexpected there... – rodrigo Mar 10 '15 at 11:17
  • As @rodrigo says, you're not checking how much data was received - nor are you null-terminating the buffer, which means your call to `string::append` could easily run off the end of the buffer. – Jonathan Potter Mar 10 '15 at 11:27
  • @rodrigo @jonathan-potter The buffer is null-terminated because I'm zeroring it at the start of each loop (`memset`) and calling ReadFile() with the size of the buffer - 1. Also checking is IMHO not nesscessary because ReadFile is returning false and setting the error number to `ERROR_MORE_DATA`. But even then when I changed the while condition to `while(bytes_left_to_read)` I've got the same behavoiur. (And I know there is text on the pipe left) – Superlokkus Mar 10 '15 at 11:55
  • Two things: (1) you should only be getting ERROR_MORE_DATA if the pipe is in message mode, which it shouldn't be; (2) the code as shown will terminate the child before it has finished writing to the pipe, unless the pipe's buffer is large enough to hold all of the content. Note that the requested pipe buffer size is advisory; the actual buffer size might be lower than expected. – Harry Johnston Mar 10 '15 at 21:14
  • @HarryJohnston Yeah you were right with (2), too bad the microsoft documentation never goes into detail about that (AFAIK). Got it by myself but thank you anyway :) ! – Superlokkus Mar 11 '15 at 10:40

1 Answers1

0

Yeah looks like I got the ordering wrong, but since I don't want to wait forever on a maybe hanging process, I'm reading in the pipes now by threads I start my read_pipes functions before the waiting with timeout, with std::async. Snipett as example:

    if (!CloseHandle(mddb_importer_stderr_w)) throw std::runtime_error("Can not create mddb_importer pipes");


auto err = std::async(std::launch::async, read_pipes, mddb_importer_stderr_r);
auto out = std::async(std::launch::async, read_pipes, mddb_importer_stdout_r);

if (WaitForSingleObject(mddb_importer_info.hProcess, 60000) == WAIT_TIMEOUT) {
    //Kill the importer, probably hangs
    TerminateProcess(mddb_importer_info.hProcess, EXIT_FAILURE);
}
GetExitCodeProcess(mddb_importer_info.hProcess, &mddb_importer_return);
//assert (mddb_importer_return != STILL_ACTIVE);

const std::string err_string = err.get();
Superlokkus
  • 4,731
  • 1
  • 25
  • 57