Pipe issue on LLDB Windows?

Hi,

I have been seeing an issue with the refactored pipe support changes on Windows using _pipe().

This is specifically at the ::select function in ConnectionFileDescriptor::BytesAvailable().
On Windows, the ::select fails if the pipe file descriptor is also included and so the connection fails.

http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ConnectionFileDescriptor.cpp?view=markup

Wanted to ask if anybody else on Windows is seeing any such issue?

Thanks,
Deepak

This is a known issue. But I don’t think this is a regression. It’s just always been this way. Basically on Windows, select() only deals with sockets. It doesn’t work with pipes, files, or anything else. In other words, ConnectionFileDescriptor is just fundamentally broken on Windows. I recently pushed a large refactor to the socket logic in ConnectionFileDescriptor which is aimed at addressing this. But it’s only one step of what I think will be a long process to get ConnectionFileDescriptor working on Windows.

Sorry, hit enter too soon. I have been thinking about next steps for fixing this in the longer term. I think the way to go is that on Windows, ConnectionFileDescriptor shouldn’t even use select at all, nor should it use the command pipe. The purpose of the command pipe seems to be so that various interrupt commands can be sent to interrupt the select, and then terminate the connection or something else so that it doesn’t block forever.

On Windows, the closest equivalent to select is WaitForMultipleObjects. So I think on Windows we need to switch to using WFMO instead of select(). The command pipe will be replaced by various event objects, which the user will set according to which interruption command they want to send. WFMO doesn’t accept sockets though, so we need to call WSAEventSelect() to get an event handle corresponding to read/write operations on the socket.

There’s numerous other portability issues with this class currently, most notably that select() on windows doesn’t set errno

Ah, so it is a known issue. Thanks, got it now.
We recently did a merge which brought in the new changes from upstream.

Isn’t the ::select used in ConnectionFileDescriptor to wait till input is available?
Not just from the command pipe, but also from the sockets.

Yes, that’s correct. It’s just that on Windows, select() won’t accept a pipe, it will just return an error. Which we also don’t handle correctly, it turns out, since we assume that the error it’s returning is an errno, which on Windows it’s not.

That said, I’m pretty sure ConnectionFileDescriptor has never worked on Windows. Are you seeing a regression in behavior? i.e. something worked before you merged, but now it doesn’t?

Yes, we have a regression in behaviour.

Before _pipe() was added, pipes were ignored on Windows. They were not added to the read_fds list for select(). So, the BytesAvailable() did not error.

However, now since pipes are created, it is added to the read_fds list and then select() fails like you said, because it is not a socket. It fails with a WSAENOTSOCK error. Due to this, the BytesAvailable() errors out and no data is read from the remote socket connection.

I only found this after the merge, as we were not able to read data from our remote debug stub.

Ahh, I see. Can you just put an #ifdef _WIN32 that doesn’t add the pipe_fd to the fd_set for Windows? It’s broken either way, but at least this way it’s less broken.

Yeah, I had done that, thanks.
Just wanted to find out if it’s just us with the issue as didn’t see anything mentioned on the list.

No worries. Do you want to commit that fix by itself, or should I?

Could you please make the change, I am not at my work machine now.
Or I can fix it tomorrow.

Thanks!

No problem, I’ll do it later todya.