Assert in libuv about uvw HOT 17 CLOSED

I just checked the libuv code, it would assert() :)

from uvw.

It looks like the handle for the pipe is gone for some reasons.
uvw forwards the request to the underlying library and nothing more in case of a read. Moreover, the read is part of the stream class, not of the pipe itself. Therefore it's exactly the same read member function you have on a tcp handles and so on.

By looking at the code of libuv, I see that it sets the descriptor to -1 when the handle is closed (here is the link to the function uv__stream_close). uv__stream_close is invoked by uv__pipe_close. uv__pipe_close is invoked by uv_close. Finally, uv_close is invoked in uvw by means of the close member function of a handle.
On the other side, uvw never invokes explicitly close on a handle. It's in charge to the user to invoke it explicitly to close a handle. The documentation is pretty clear on this point.

Could it happen that you close the handle for some reasons (as an example, in case of errors) and then you try to reuse it when you should create a new one from scratch instead?

from uvw.

Actually I'm doing this:

    pipe = uvw::Loop::getDefault()->resource<uvw::PipeHandle>();
    process_exe->stdio(static_cast<uvw::FileHandle>(0), uvw::ProcessHandle::StdIO::IGNORE_STREAM);

    uv_stdio_flags f = (uv_stdio_flags)(UV_CREATE_PIPE | UV_READABLE_PIPE);
    uvw::Flags<uvw::ProcessHandle::StdIO> ff(f);
    process_exe->stdio(*pipe, ff);

    //When pipe is closed, remove it and close it
    pipe->once<uvw::EndEvent>([](const uvw::EndEvent &, auto &cl) { cl.close(); });
    pipe->on<uvw::DataEvent>([this](uvw::DataEvent &ev, auto &)
    {
        [...]
    });

    Utils::CStrArray arr(cmd);
    process_exe->spawn(arr.at(0), arr.data());
    pipe->read();

The only place where I'm closing the handle is for the uvw::EndEvent. I'm creating a new uvw::PipeHandle everytime I need to start the process. Is it ok like that?

from uvw.

You should stop or close a handle in case of errors during a read too.
uvw doesn't do it internally, but libuv is explicit about that:

The callee is responsible for stopping closing the stream when an error happens by calling uv_read_stop() or uv_close(). Trying to read from the stream again is undefined.

Unfortunately the pipe shares part of its API with the stream and the documentation isn't that easy to follow.

It could happen that you're getting an error on your handle for unknown reasons (try to analyze the error code for that) and you don't close it.
Documentation mentions an undefined behavior, so who knows if this is the case.

After all, you are experiencing the problem during a read on an invalid descriptor.
It fits pretty well with the definition of undefined behavior. :-)

from uvw.

Actually, from here I see that your are ignoring errors on the pipe handle.
Probably I'd start listening to the ErrorEvent to know if something goes wrong during a read and debug it.

from uvw.

I'm marking the issue as invalid for it doesn't look like it's due to a bug in uvw.
Keep me updated so that I can change the labels or close the issue if you find the root of the problem.
Thank you.

from uvw.

Thanks for all your pointers.

I tried to listen to ErrorEvent for the pipe, it did not trigger before asserting.

from uvw.

Interesting. Neither the ErrorEvent nor the EndEvent trigger before the assert?

from uvw.

No, neither ErrorEvent nor the EndEvent are triggered.

I did some more test and found out that

process_exe->spawn(arr.at(0), arr.data());

is failing due to too many open files error. The problem was that the ErrorEvent is triggered directly in a lambda.
I think this API in uvw is not so nice. In libuv uv_spawn the error is directly returned, but in uvw the error has to be catched in an ErrorEvent, which is a bit more painful to write.

I close this bug, as it's not relevant.
Thank you for your anwsers.

from uvw.

@raoulh Any suggestion to change the API and make it more usable is welcome.
You are using the library in a real world project, so your feedback are highly appreciated.
Feel free to drop in a replacement for the current API and we can discuss it together.

from uvw.

It would have been better to be able to do something like:

exe = uvw::Loop::getDefault()->resource<uvw::ProcessHandle>();
exe->once<uvw::ErrorEvent>([this](const uvw::ErrorEvent &ev, auto &)
{
    process_exe->close();
});

[...] //setup pipe

if (!exe->spawn())
{
   //early failure, ErrorEvent is not called
   std::cout << "fail to start process: " << exe->errorMsg() << std::endl;
   exe->close();
}
else
{
   pipe->read();
}

But I don't know if it's a good idea to differenciate between error types. It makes sense when like in this example the pipe->read() call must be done only if exe->spawn() returns ok.

from uvw.

I also found why I was leaking fds... I did not close() the handle somewhere in case of ErrorEvent... That was not clear until you explained it here.
Thanks

from uvw.

You are welcome. You know, you're skilled and when you pop up with an issue it's usually an interesting problem from which I can learn something. ;-)

But I don't know if it's a good idea to differenciate between error types. It makes sense when like in this example the pipe->read() call must be done only if exe->spawn() returns ok.

I suggest to not remove the ErrorEvent on the spawn to avoid breaking the current API.
However we could just add a return type to the function and both trigger the event and return the correct value.
This way both the approaches are possible and one can choose for the best time by time.

What about?

from uvw.

Glad to help :) That's also why I posted here in the first place.

About the return value and the ErrorEvent, that would mean the close() has to be done in the ErrorEvent only and not also after an error is returned from spawn (like in my example), or close() would be called twice, right?

from uvw.

uvw never calls close and it won't do it any time soon.
That's because it stays true to how libuv works and with libuv it's in charge to the user to close a handle.
Moreover, from within the spawn member function I cannot say neither if the caller is getting the return value or if it's interested in the event nor what it's going to do with this information.

As a side note, did you see the walk member function of the Loop?
Probably it could help if you want to perform a cleanup once each N ticks.

from uvw.

Yes I know that libuv/uvw does not do any close itself. I was talking about my usage example above.

The walk function is nice, but I cannot use for cleaning things in my project.

from uvw.

Oho, I got it now. Sorry. Yeah, right, it's better not to call close twice actually.
Honestly, libuv documentation isn't explicit about the results, but I wouldn't risk if possible. ;-)

from uvw.