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.

On 7 Aug 2014, at 22:15, Zachary Turner <[email protected]> wrote:

> 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?
> 
> 
> On Thu, Aug 7, 2014 at 2:06 PM, Deepak Panickal <[email protected]> wrote:
> 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.
> 
> On 7 Aug 2014, at 21:38, Zachary Turner <[email protected]> wrote:
> 
>> 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
>> 
>> 
>> On Thu, Aug 7, 2014 at 1:34 PM, Zachary Turner <[email protected]> wrote:
>> 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.
>> 
>> 
>> On Thu, Aug 7, 2014 at 1:01 PM, Deepak Panickal <[email protected]> wrote:
>> 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
>> 
>> 
>> _______________________________________________
>> lldb-dev mailing list
>> [email protected]
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
>> 
>> 
> 
> 

_______________________________________________
lldb-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev

Reply via email to