From 3a1f8f2c6cabc38fcdba6a6eaa8e1018e0d9f0b4 Mon Sep 17 00:00:00 2001 From: "Yaroshenko, Paul" Date: Wed, 9 Apr 2025 19:38:24 +0300 Subject: [PATCH 1/6] Ignore pyCharm stuff and symlink for local testing. Convert _win32 to ctype --- .gitignore | 2 + src/namedpipe/_win32.py | 122 +++++++++++++++++++++++----------------- 2 files changed, 72 insertions(+), 52 deletions(-) diff --git a/.gitignore b/.gitignore index c914563..836df5d 100644 --- a/.gitignore +++ b/.gitignore @@ -171,3 +171,5 @@ cython_debug/ # Built Visual Studio Code Extensions *.vsix +/.idea +/tests/namedpipe diff --git a/src/namedpipe/_win32.py b/src/namedpipe/_win32.py index 36baf44..b612a57 100644 --- a/src/namedpipe/_win32.py +++ b/src/namedpipe/_win32.py @@ -1,4 +1,5 @@ -import win32pipe, win32file, win32api, winerror, win32con, pywintypes +import ctypes +from ctypes import wintypes from os import path import io from typing import TypeVar, NewType, Literal, IO, Optional, Union @@ -6,8 +7,25 @@ WritableBuffer = TypeVar("WritableBuffer") PyHANDLE = NewType("PyHANDLE", int) +# Define global constants +PIPE_ACCESS_DUPLEX = 0x00000003 +PIPE_ACCESS_INBOUND = 0x00000001 +PIPE_ACCESS_OUTBOUND = 0x00000002 +PIPE_TYPE_BYTE = 0x00000000 +PIPE_READMODE_BYTE = 0x00000000 +PIPE_WAIT = 0x00000000 +PIPE_UNLIMITED_INSTANCES = 0xFFFFFFFF +ERROR_PIPE_CONNECTED = 535 +ERROR_BROKEN_PIPE = 109 +ERROR_MORE_DATA = 234 +ERROR_IO_PENDING = 997 +FORMAT_MESSAGE_FROM_SYSTEM = 0x00001000 +INVALID_HANDLE_VALUE = wintypes.HANDLE(-1).value + id = 0 +def _wt(value: int) -> wintypes.DWORD: + return wintypes.DWORD(value) def _name_pipe(): global id @@ -23,11 +41,18 @@ def _name_pipe(): def _win_error(code=None): if not code: - code = win32api.GetLastError() - return OSError( - f"[OS Error {code}] {win32api.FormatMessage(win32con.FORMAT_MESSAGE_FROM_SYSTEM,0,code,0,None)}" + code = ctypes.GetLastError() + buf = ctypes.create_unicode_buffer(256) + ctypes.FormatMessageW( + FORMAT_MESSAGE_FROM_SYSTEM, + None, + code, + 0, + buf, + len(buf), + None ) - + return OSError(f"[OS Error {code}] {buf.value}") class NPopen: def __init__( @@ -72,30 +97,26 @@ def __init__( ) self._bufsize = -1 if txt_mode and bufsize == 1 else bufsize - if self._rd and self._wr: - access = win32pipe.PIPE_ACCESS_DUPLEX + access = _wt(PIPE_ACCESS_DUPLEX) elif self._rd: - access = win32pipe.PIPE_ACCESS_INBOUND + access = _wt(PIPE_ACCESS_INBOUND) elif self._wr: - access = win32pipe.PIPE_ACCESS_OUTBOUND + access = _wt(PIPE_ACCESS_OUTBOUND) else: raise ValueError("Invalid mode") # TODO: assess options: FILE_FLAG_WRITE_THROUGH, FILE_FLAG_OVERLAPPED, FILE_FLAG_FIRST_PIPE_INSTANCE + pipe_mode = _wt(PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT) - pipe_mode = ( - win32pipe.PIPE_TYPE_BYTE - | win32pipe.PIPE_READMODE_BYTE - | win32pipe.PIPE_WAIT - ) # TODO: assess options: PIPE_WAIT, PIPE_NOWAIT, PIPE_ACCEPT_REMOTE_CLIENTS, PIPE_REJECT_REMOTE_CLIENTS - max_instances = win32pipe.PIPE_UNLIMITED_INSTANCES # 1 - buffer_size = 0 - timeout = 0 + max_instances = _wt(PIPE_UNLIMITED_INSTANCES) + buffer_size = _wt(0) + timeout = _wt(0) + # "open" named pipe - self._pipe = win32pipe.CreateNamedPipe( + self._pipe = ctypes.windll.kernel32.CreateNamedPipeW( self._path, access, pipe_mode, @@ -105,7 +126,7 @@ def __init__( timeout, None, ) - if self._pipe == win32file.INVALID_HANDLE_VALUE: + if self._pipe == INVALID_HANDLE_VALUE: raise _win_error() @property @@ -114,28 +135,28 @@ def path(self): return self._path def __str__(self): - # return the path return self._path def close(self): - # close named pipe + """Close the named pipe. + A closed pipe cannot be used for further I/O operations. `close()` may + be called more than once without error. + """ if self.stream is not None: self.stream.close() self.stream = None if self._pipe is not None: - if win32file.CloseHandle(self._pipe): + if not ctypes.windll.kernel32.CloseHandle(self._pipe): raise _win_error() self._pipe = None def wait(self): - + """Wait for the pipe to open (the other end to be opened) and return file object to read/write.""" if not self._pipe: raise RuntimeError("pipe has already been closed.") - - # wait for the pipe to open (the other end to be opened) and return fileobj to read/write - if win32pipe.ConnectNamedPipe(self._pipe, None): - code = win32api.GetLastError() - if code != 535: # ERROR_PIPE_CONNECTED (ok, just indicating that the client has already connected)(Issue#3) + if not ctypes.windll.kernel32.ConnectNamedPipe(self._pipe, None): + code = ctypes.GetLastError() + if code != ERROR_PIPE_CONNECTED: # (ok, just indicating that the client has already connected)(Issue#3) raise _win_error(code) # create new io stream object @@ -179,7 +200,7 @@ def writable(self) -> bool: class Win32RawIO(io.RawIOBase): """Raw I/O stream layer over open Windows pipe handle. - `handle` is an open ``pywintypes.PyHANDLE`` object (from ``pywin32`` package) to + `handle` is an open Windows ``HANDLE`` object (from ``ctype`` package) to be wrapped by this class. Specify the read and write modes by boolean flags: ``rd`` and ``wr``. @@ -187,7 +208,7 @@ class Win32RawIO(io.RawIOBase): def __init__(self, handle: PyHANDLE, rd: bool, wr: bool) -> None: super().__init__() - self.handle = handle # PyHANDLE: Underlying Windows handle. + self.handle = handle # Underlying Windows handle. self._readable: bool = rd self._writable: bool = wr @@ -211,7 +232,7 @@ def close(self) -> None: if self.closed: return if self.handle is not None: - win32file.CloseHandle(self.handle) + ctypes.windll.kernel32.CloseHandle(self.handle) self.handle = None super().close() @@ -224,23 +245,16 @@ def readinto(self, b: WritableBuffer) -> Union[int, None]: assert self._readable size = len(b) - nread = 0 - while nread < size: - try: - hr, res = win32file.ReadFile(self.handle, size - nread) - if hr in (winerror.ERROR_MORE_DATA, winerror.ERROR_IO_PENDING): - raise _win_error(hr) - except pywintypes.error as e: - if e.args[0] == 109: # broken pipe error - break - else: - raise e - if not len(res): - break - nnext = nread + len(res) - b[nread:nnext] = res - nread = nnext - return nread + nread = _wt(0) + buf = (ctypes.c_char * size).from_buffer(b) + + success = ctypes.windll.kernel32.ReadFile(self.handle, buf, size, ctypes.byref(nread), None) + if not success: + code = ctypes.GetLastError() + if code not in (ERROR_MORE_DATA, ERROR_IO_PENDING): + raise _win_error(code) + + return nread.value def write(self, b): """Write buffer ``b`` to file, return number of bytes written. @@ -250,7 +264,11 @@ def write(self, b): assert self.handle is not None # show type checkers we already checked assert self._writable - hr, n_written = win32file.WriteFile(self.handle, b) - if hr: - raise _win_error(hr) - return n_written + size = len(b) + nwritten = wintypes.DWORD(0) + buf = (ctypes.c_char * size).from_buffer_copy(b) + if not ctypes.windll.kernel32.WriteFile(self.handle, buf, size, ctypes.byref(nwritten), None): + code = ctypes.GetLastError() + raise _win_error(code) + + return nwritten.value From 0a6f9add7ea5240d50096b9a8dd3039f7344d276 Mon Sep 17 00:00:00 2001 From: "Yaroshenko, Paul" <81985296+samepaul@users.noreply.github.com> Date: Wed, 9 Apr 2025 21:01:08 +0300 Subject: [PATCH 2/6] Adjustments after AI to make it actually work --- src/namedpipe/_win32.py | 45 +++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/src/namedpipe/_win32.py b/src/namedpipe/_win32.py index b612a57..0042366 100644 --- a/src/namedpipe/_win32.py +++ b/src/namedpipe/_win32.py @@ -20,7 +20,7 @@ ERROR_MORE_DATA = 234 ERROR_IO_PENDING = 997 FORMAT_MESSAGE_FROM_SYSTEM = 0x00001000 -INVALID_HANDLE_VALUE = wintypes.HANDLE(-1).value +INVALID_HANDLE_VALUE = -1 id = 0 @@ -41,18 +41,8 @@ def _name_pipe(): def _win_error(code=None): if not code: - code = ctypes.GetLastError() - buf = ctypes.create_unicode_buffer(256) - ctypes.FormatMessageW( - FORMAT_MESSAGE_FROM_SYSTEM, - None, - code, - 0, - buf, - len(buf), - None - ) - return OSError(f"[OS Error {code}] {buf.value}") + code = ctypes.get_last_error() + return ctypes.WinError(code) class NPopen: def __init__( @@ -69,6 +59,7 @@ def __init__( if not isinstance(bufsize, int): raise TypeError("bufsize must be an integer") + self.kernel32 = ctypes.WinDLL('kernel32', use_last_error=True) self.stream: Union[IO, None] = None # I/O stream of the pipe self._path = _name_pipe() if name is None else rf"\\.\pipe\{name}" self._rd = any(c in mode for c in "r+") @@ -110,13 +101,12 @@ def __init__( # TODO: assess options: PIPE_WAIT, PIPE_NOWAIT, PIPE_ACCEPT_REMOTE_CLIENTS, PIPE_REJECT_REMOTE_CLIENTS - max_instances = _wt(PIPE_UNLIMITED_INSTANCES) - buffer_size = _wt(0) + max_instances = _wt(1) # PIPE_UNLIMITED_INSTANCES returns 'invalid params'. Pipes are point-to-point anyway + buffer_size = _wt(65536) # in all examples online provide 64KB buffer. 0 doesn't fail CreateNamedPipeW, but maybe performance better? timeout = _wt(0) - # "open" named pipe - self._pipe = ctypes.windll.kernel32.CreateNamedPipeW( + h = self.kernel32.CreateNamedPipeW( self._path, access, pipe_mode, @@ -126,8 +116,9 @@ def __init__( timeout, None, ) - if self._pipe == INVALID_HANDLE_VALUE: + if h == INVALID_HANDLE_VALUE: raise _win_error() + self._pipe = h @property def path(self): @@ -146,7 +137,7 @@ def close(self): self.stream.close() self.stream = None if self._pipe is not None: - if not ctypes.windll.kernel32.CloseHandle(self._pipe): + if not self.kernel32.CloseHandle(self._pipe): raise _win_error() self._pipe = None @@ -154,8 +145,8 @@ def wait(self): """Wait for the pipe to open (the other end to be opened) and return file object to read/write.""" if not self._pipe: raise RuntimeError("pipe has already been closed.") - if not ctypes.windll.kernel32.ConnectNamedPipe(self._pipe, None): - code = ctypes.GetLastError() + if not self.kernel32.ConnectNamedPipe(self._pipe, None): + code = ctypes.get_last_error() if code != ERROR_PIPE_CONNECTED: # (ok, just indicating that the client has already connected)(Issue#3) raise _win_error(code) @@ -208,6 +199,7 @@ class Win32RawIO(io.RawIOBase): def __init__(self, handle: PyHANDLE, rd: bool, wr: bool) -> None: super().__init__() + self.kernel32 = ctypes.WinDLL('kernel32', use_last_error=True) self.handle = handle # Underlying Windows handle. self._readable: bool = rd self._writable: bool = wr @@ -232,7 +224,7 @@ def close(self) -> None: if self.closed: return if self.handle is not None: - ctypes.windll.kernel32.CloseHandle(self.handle) + self.kernel32.CloseHandle(self.handle) self.handle = None super().close() @@ -248,9 +240,9 @@ def readinto(self, b: WritableBuffer) -> Union[int, None]: nread = _wt(0) buf = (ctypes.c_char * size).from_buffer(b) - success = ctypes.windll.kernel32.ReadFile(self.handle, buf, size, ctypes.byref(nread), None) + success = self.kernel32.ReadFile(self.handle, buf, size, ctypes.byref(nread), None) if not success: - code = ctypes.GetLastError() + code = ctypes.get_last_error() if code not in (ERROR_MORE_DATA, ERROR_IO_PENDING): raise _win_error(code) @@ -267,8 +259,7 @@ def write(self, b): size = len(b) nwritten = wintypes.DWORD(0) buf = (ctypes.c_char * size).from_buffer_copy(b) - if not ctypes.windll.kernel32.WriteFile(self.handle, buf, size, ctypes.byref(nwritten), None): - code = ctypes.GetLastError() - raise _win_error(code) + if not self.kernel32.WriteFile(self.handle, buf, size, ctypes.byref(nwritten), None): + raise _win_error() return nwritten.value From 93f4577eb74ee7d942d3c2069e6d6a750a026f23 Mon Sep 17 00:00:00 2001 From: "Yaroshenko, Paul" <81985296+samepaul@users.noreply.github.com> Date: Wed, 9 Apr 2025 21:20:11 +0300 Subject: [PATCH 3/6] Is this correct? Not sure, never dealt with delivering public Python modules --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d0660e2..52812c2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,7 +22,7 @@ classifiers = [ ] dynamic = ["version"] requires-python = ">=3.9" -dependencies = ["pywin32;platform_system=='Windows'"] +dependencies = [] [project.urls] Repository = "https://github.com/python-ffmpegio/python-namedpipe" From 4898c7cd60baac4d84bb213a375fe6e901475353 Mon Sep 17 00:00:00 2001 From: "Yaroshenko, Paul" <81985296+samepaul@users.noreply.github.com> Date: Wed, 9 Apr 2025 21:42:08 +0300 Subject: [PATCH 4/6] Zero-length buffers and no exception on ERROR_BROKEN_PIPE --- src/namedpipe/_win32.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/namedpipe/_win32.py b/src/namedpipe/_win32.py index 0042366..b1b6bc7 100644 --- a/src/namedpipe/_win32.py +++ b/src/namedpipe/_win32.py @@ -102,7 +102,7 @@ def __init__( # TODO: assess options: PIPE_WAIT, PIPE_NOWAIT, PIPE_ACCEPT_REMOTE_CLIENTS, PIPE_REJECT_REMOTE_CLIENTS max_instances = _wt(1) # PIPE_UNLIMITED_INSTANCES returns 'invalid params'. Pipes are point-to-point anyway - buffer_size = _wt(65536) # in all examples online provide 64KB buffer. 0 doesn't fail CreateNamedPipeW, but maybe performance better? + buffer_size = _wt(0) timeout = _wt(0) # "open" named pipe @@ -243,7 +243,10 @@ def readinto(self, b: WritableBuffer) -> Union[int, None]: success = self.kernel32.ReadFile(self.handle, buf, size, ctypes.byref(nread), None) if not success: code = ctypes.get_last_error() - if code not in (ERROR_MORE_DATA, ERROR_IO_PENDING): + # ERROR_MORE_DATA - not big deal, will read next time + # ERROR_IO_PENDING - should not happen, unless use OVERLAPPING, which we don't so far + # ERROR_BROKEN_PIPE - pipe was closed from other end. While it is an error, test seemingly expects to receive 0 instead of exception + if code not in (ERROR_MORE_DATA, ERROR_IO_PENDING, ERROR_BROKEN_PIPE): raise _win_error(code) return nread.value From a02824db8434479ff3b2a5ea6e3da7bd8196220b Mon Sep 17 00:00:00 2001 From: "Yaroshenko, Paul" <81985296+samepaul@users.noreply.github.com> Date: Wed, 9 Apr 2025 22:19:52 +0300 Subject: [PATCH 5/6] pytest passing --- src/namedpipe/_win32.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/namedpipe/_win32.py b/src/namedpipe/_win32.py index b1b6bc7..b88f813 100644 --- a/src/namedpipe/_win32.py +++ b/src/namedpipe/_win32.py @@ -137,8 +137,7 @@ def close(self): self.stream.close() self.stream = None if self._pipe is not None: - if not self.kernel32.CloseHandle(self._pipe): - raise _win_error() + self.kernel32.CloseHandle(self._pipe) self._pipe = None def wait(self): @@ -260,9 +259,9 @@ def write(self, b): assert self.handle is not None # show type checkers we already checked assert self._writable size = len(b) - nwritten = wintypes.DWORD(0) + nwritten = _wt(0) buf = (ctypes.c_char * size).from_buffer_copy(b) - if not self.kernel32.WriteFile(self.handle, buf, size, ctypes.byref(nwritten), None): + if not self.kernel32.WriteFile(self.handle, buf, _wt(size), ctypes.byref(nwritten), None): raise _win_error() return nwritten.value From 934c874439e7a8bb403c53150806dbabcaa821a5 Mon Sep 17 00:00:00 2001 From: Takeshi Ikuma Date: Wed, 9 Apr 2025 21:40:00 -0500 Subject: [PATCH 6/6] track all files in tests --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 836df5d..5af2999 100644 --- a/.gitignore +++ b/.gitignore @@ -172,4 +172,3 @@ cython_debug/ # Built Visual Studio Code Extensions *.vsix /.idea -/tests/namedpipe