[Bug 58745] New: PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

Bug ID: 58745
           Summary: PeekNamedPipe() unsuccessful on stdin
           Product: Wine
           Version: 10.10
          Hardware: x86-64
                OS: FreeBSD
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: kernel32
          Assignee: [e-mail]
          Reporter: [e-mail]

Created attachment 79378
  --> http://bugs.winehq.org/attachment.cgi?id=79378
Test program for PeekNamedPipe() bug

Trying to debug a glib2 problem occurring under Wine, I encountered what is
obviously a bug in Wine itself. See the attached test program.

This is what I observed on Wine:

# wine peeknamedpipe.exe < somefile
stat: console=0 fifo=0
ReadFile(stdin): 1
PeekNamedPipe(stdin): 0

# echo TEST | wine peeknamedpipe.exe
stat: console=0 fifo=4096
ReadFile(stdin): 1
PeekNamedPipe(stdin): 0

On a real Windows 10 however it behaves differently:
> peeknamedpipe.exe < somefile

stat: console=0 fifo=0
ReadFile(stdin): 1
PeekNamedPipe(stdin): 0
> echo TEST | peeknamedpipe.exe

stat: console=0 fifo=4096
ReadFile(stdin): 1
PeekNamedPipe(stdin): 1

In other words, PeekNamedPipe() returns FALSE on stdin when piping from another
process instead of TRUE, which is returned by a real Windows 10.
somefile needs to be some non-empty file of course.

This was tested both on Wine 10.0 and 10.10 (wine-devel) from the FreeBSD ports
tree.

I filed this in kernel32 since PeekNamedPipe() is part of Kernel32.dll (at
least according to the Microsoft documentation).

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

Eric Pouech  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Eric Pouech  ---
there are actually two different issues behind this, depending which shell
you're using for the pipe command:
- if it's run from a unix shell, then the pipe used in a unix one, which is
imported inside wine, but which doesn't have all the Win32 semantics (like
shown here with PeekNamedPipe); and I'm not sure it's something we want to or
can improve
- if it's run from cmd.exe, then currently the pipe isn't a real pipe (the lhs
command is run, its output saved into a file; then rhs command is run, with
file as input) ; see https://bugs.winehq.org/show_bug.cgi?id=48027 for another
example. this is been worked upon and should be fixed soon

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

--- Comment #2 from Robin Haberkorn  ---
(In reply to Eric Pouech from comment #1)
> - if it's run from a unix shell, then the pipe used in a unix one, which is
>
> imported inside wine, but which doesn't have all the Win32 semantics (like
> shown here with PeekNamedPipe); and I'm not sure it's something we want to
> or can improve

If I run the `echo TEST | ./peeknamedpipe.exe` in an MSYS shell under Windows
10, ie. a bash ported to Windows, I get exactly the same output as under
cmd.exe. Effectively you can have a console application reading from stdin that
works correctly in MSYS under Windows, but won't run correctly on a FreeBSD
shell (be it bash) with Wine, e.g. in a quasi-msys2 [1] environment. Which is
exactly how I noticed the difference in behavior.

But if you say it can't be fixed in Wine the glib people would be willing to
add a workaround. They want a clear statement, though.

I suppose the only workaround for now would be using temporary files everywhere
this causes problems. (I do not have infinite streams luckily.)

[1]: https://github.com/HolyBlackCat/quasi-msys2

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

--- Comment #3 from Eric Pouech  ---
that's very unlikely that's going to be fixed in Wine

to do a proper implementation, you'd need to somehow buffer the unix pipe
stream (for eg peeknamedpipe to work) and that cannot be done on reader side
(as several processes can peek from the same Unix pipe)
this cannot be done on writer side as its a Unix process

doing that in the Wine server would be rather complex (signal state of the pipe
should be set according to the buffer state...)

That goes IMO beyond the goals of Wine (which is the emulate Windows); what's
expected here is a full integration between Unix objects and Windows objects

From your latest post: why would you need temporary files? reading from the
wrapped unix pipe with ReadFile would work just fine (I mean reading from temp
files or reading from the wrapped unix pipe should behave the same). (it
doesn't work if the app has only -from stdin- or -from-named-file' command line
options though)

I also assumed that LHS from pipe is a unix executable whereas RHS is a windows
executable (if both were win32 executables this could be executed with cmd.exe)

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

--- Comment #4 from Robin Haberkorn  ---
(In reply to Eric Pouech from comment #3)
> to do a proper implementation, you'd need to somehow buffer the unix pipe
> stream (for eg peeknamedpipe to work) and that cannot be done on reader side
> (as several processes can peek from the same Unix pipe)
> this cannot be done on writer side as its a Unix process

But couldn't you just return TRUE when peeking 0 elements on the pipe? Even if
you do not allow peeking more than 0 elements, this would already improve
compatibility.
> From your latest post: why would you need temporary files? reading from the
> wrapped unix pipe with ReadFile would work just fine (I mean reading from
> temp files or reading from the wrapped unix pipe should behave the same).

Because I have code using glib and as long as we don't have a workaround in
glib itself, I cannot read from stdin using glib APIs (GIOChannel). They do a
zero-peek on the pipe to determine whether it is readable. [1] As long as this
is a given, AFAIK I could only work around the issue on my side - where I have
full control - with temporary files.

Let's suppose Wine is not changed and I would like to work around this issue in
glib, what would you suppose to do in order to determine whether the pipe is
readable? Is using PeekNamedPipe() instead of ReadFile() for FIFOs necessary at
all? I suppose they had some reason to write it like this in the first place.

[1] https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/giowin32.c#L1780

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

--- Comment #5 from Eric Pouech  ---
(In reply to Robin Haberkorn from comment #4)
> (In reply to Eric Pouech from comment #3)
> > to do a proper implementation, you'd need to somehow buffer the unix pipe
> > stream (for eg peeknamedpipe to work) and that cannot be done on reader side
> > (as several processes can peek from the same Unix pipe)
> > this cannot be done on writer side as its a Unix process
>
> But couldn't you just return TRUE when peeking 0 elements on the pipe? Even
> if you do not allow peeking more than 0 elements, this would already improve
> compatibility.

I agree that peek:ing 0 elements is easier to implement as it doesn't require
buffering

one quick workaround is to fallback to ReadFile when PeekNamedPipe fails

in Wine (quick & dirty, didn't test it, could help diagnose if there are other
problems):
diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c
index 6c69a0ab88f..bb102c7e322 100644
--- a/dlls/kernelbase/sync.c
+++ b/dlls/kernelbase/sync.c
@@ -1568,8 +1568,8 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetNamedPipeInfo( HANDLE
pipe, LPDWORD flags, LPDW
 /***********************************************************************
  *           PeekNamedPipe   (kernelbase.@)
  */
-BOOL WINAPI DECLSPEC_HOTPATCH PeekNamedPipe( HANDLE pipe, LPVOID out_buffer,
DWORD size,
-                                             LPDWORD read_size, LPDWORD avail,
LPDWORD message )
+static BOOL PeekNamedPipe2( HANDLE pipe, LPVOID out_buffer, DWORD size,
+                            LPDWORD read_size, LPDWORD avail, LPDWORD message
)
 {
     FILE_PIPE_PEEK_BUFFER local_buffer;
     FILE_PIPE_PEEK_BUFFER *buffer = &local_buffer;
@@ -1600,6 +1600,19 @@ BOOL WINAPI DECLSPEC_HOTPATCH PeekNamedPipe( HANDLE
pipe, LPVOID out_buffer, DWO
     return !status;
 }

+BOOL WINAPI DECLSPEC_HOTPATCH PeekNamedPipe( HANDLE pipe, LPVOID out_buffer,
DWORD size,
+                                             LPDWORD read_size, LPDWORD avail,
LPDWORD message )
+{
+    if (!PeekNamedPipe2(pipe, out_buffer, size, read_size, avail, message) &&
GetLastError() != ERROR_BROKEN_PIPE &&
+        size == 0 && avail == NULL && message == NULL)
+    {
+        DWORD gle = GetLastError();
+        if (ReadFile(pipe, out_buffer, 0, read_size, NULL)) return TRUE;
+        SetLastError(gle);
+        return FALSE;
+    }
+    return TRUE;
+}

/***********************************************************************
  *           SetNamedPipeHandleState  (kernelbase.@)

in glib:
in the _S_IFIFO case, I'd do something like:
(it's possible that PeekNamedPipe has been used to work around blocking reads
on pipe:s)
so perhaps a very simple hack would be to add after readable and writable have
been set something like:
if (GetFileType(pipe) == FILE_TYPE_PIPE && !channel->is_readable &&
!channel->is_writable) channel->is_readable = 1;
(it's broken if the pipe is neither readable nor writable, but detects if the
pipe handle is still valid, and doesn't use ReadFile in case of blocking reads)
also not tested
(or falling back to ReadFile)

neither options look nice to me and really look like dirty workarounds
there are other ways to get handle attributes, but not from kernel32/kernelbase
IIRC

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

--- Comment #6 from Robin Haberkorn  ---
(In reply to Eric Pouech from comment #5)
> in glib:
> in the _S_IFIFO case, I'd do something like:
> (it's possible that PeekNamedPipe has been used to work around blocking
> reads on pipe:s)

Yes, that's what the glib maintainers say.
> so perhaps a very simple hack would be to add after readable and writable
> have been set something like:
> if (GetFileType(pipe) == FILE_TYPE_PIPE && !channel->is_readable &&
> !channel->is_writable) channel->is_readable = 1;
> (it's broken if the pipe is neither readable nor writable, but detects if
> the pipe handle is still valid, and doesn't use ReadFile in case of blocking
> reads)
> also not tested
> (or falling back to ReadFile)

Somebody is suggesting to DuplicateHandle() and set the PIPE_NOWAIT mode before
trying a zero ReadFile().

[Bug 58745] PeekNamedPipe() unsuccessful on stdin

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58745

--- Comment #7 from Eric Pouech  ---
> Somebody is suggesting to DuplicateHandle() and set the PIPE_NOWAIT mode
> before trying a zero ReadFile().

quickly tested (on Wine & windows) for (anonymous) pipes, and as I feared,
PIPE_NOWAIT is an attribute of the pipe kernel object not of the handle...
so changing wait mode on the dup:ed handle also changes it on original handle
(but ReadFile on either handles wouldn't block)

(there are ways to get read & write attributes out of a handle, but they only
exist in ntdll and are not exposed to kernel32 nor kernelbase; so not sure
that's suitable for this context)