[Bug 58745] New: PeekNamedPipe() unsuccessful on stdin
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: wine-bugs(a)winehq.org Reporter: rhaberkorn(a)fmsbw.de 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). -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 Eric Pouech <eric.pouech(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eric.pouech(a)gmail.com --- Comment #1 from Eric Pouech <eric.pouech(a)gmail.com> --- 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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 --- Comment #2 from Robin Haberkorn <rhaberkorn(a)fmsbw.de> --- (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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 --- Comment #3 from Eric Pouech <eric.pouech(a)gmail.com> --- 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) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 --- Comment #4 from Robin Haberkorn <rhaberkorn(a)fmsbw.de> --- (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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 --- Comment #5 from Eric Pouech <eric.pouech(a)gmail.com> --- (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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 --- Comment #6 from Robin Haberkorn <rhaberkorn(a)fmsbw.de> --- (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(). -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 --- Comment #7 from Eric Pouech <eric.pouech(a)gmail.com> ---
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) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58745 Joel Holdsworth <joel@airwebreathe.org.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joel@airwebreathe.org.uk -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (2)
-
WineHQ Bugzilla -
WineHQ Bugzilla