Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Windows #6

Open
2 tasks
peterstory opened this issue Oct 17, 2023 · 4 comments
Open
2 tasks

Add Support for Windows #6

peterstory opened this issue Oct 17, 2023 · 4 comments
Assignees

Comments

@peterstory
Copy link
Contributor

Unit tests fail for various reasons when run on Windows.
Also, I couldn't successfully transfer a file using the GUI.

  • Update unit tests for compatibility with Windows
  • Ensure files can be transferred using the GUI
@Patrick-Ziegler
Copy link

From my testing on windows I found the following:

  • Files cannot be transferred using the GUI, Sending side will stay with the loading bar full and receiving will never receive anything
  • Through use of the debug commands, the following occurred:
    • The command pydiode --debug receive 127.0.0.1 gave seemingly no errors (although this was without sending anything
    • The command pydiode --debug send 127.0.0.1 127.0.0.1 returned the following:
DEBUG:root:chunk_max_packets=100
DEBUG:root:chunk_duration=0.0073728
DEBUG:root:PACKET_HEADER.size=5
DEBUG:root:MAX_PAYLOAD=9211
DEBUG:root:chunk_max_data_bytes=921100
ERROR:asyncio:Exception in callback _ProactorReadPipeTransport._loop_reading()
handle: <Handle _ProactorReadPipeTransport._loop_reading()>
Traceback (most recent call last):
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\proactor_events.py", line 306, in _loop_reading
    self._read_fut = self._loop._proactor.recv_into(self._sock, self._data)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\windows_events.py", line 483, in recv_into
    self._register_with_iocp(conn)
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\windows_events.py", line 704, in _register_with_iocp
    _overlapped.CreateIoCompletionPort(obj.fileno(), self._iocp, 0, 0)
OSError: [WinError 6] The handle is invalid

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\proactor_events.py", line 316, in _loop_reading
    self._fatal_error(exc, 'Fatal read error on pipe transport')
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\proactor_events.py", line 132, in _fatal_error
    self._force_close(exc)
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\proactor_events.py", line 135, in _force_close
    if self._empty_waiter is not None and not self._empty_waiter.done():
       ^^^^^^^^^^^^^^^^^^
AttributeError: '_ProactorReadPipeTransport' object has no attribute '_empty_waiter'

When i looked into this a little more, it seems there are compatibility issues with the asyncio library

See asyncio documentation on compatibility

Additionally here is an old but seemingly related github ticket. Although they are not having the exact same issue as us, there are some people in this thread that seem much more knowledgeable than me about asyncio discussing its compatibility issues with windows. Note that this thread, despite still being open, is from 2016, and asyncio has likely changed a bit since then.

Despite it not working, it is also worth mentioning This stack overflow thread regarding someone getting the same winerror 6, although it is seemingly under different circumstances. I attempted the recommended fix of adding asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) before the two occurrences of loop = asyncio.get_event_loop() within send.py. this seemingly did nothing (although as I did not write the code its possible I may have misinterpreted where this line was supposed to go.)

I was unable to find anyone who had a solution/alternative for these issues (via a quick google search and some chatting with chatGPT), but I am also not super familiar with asyncio, so that doesn't necessarily mean they don't exist.

@Patrick-Ziegler
Copy link

Patrick-Ziegler commented Mar 11, 2024

I did a little more digging into the asyncio documentation on event loops based on this particular section of the error ERROR:asyncio:Exception in callback _ProactorReadPipeTransport._loop_reading() handle: <Handle _ProactorReadPipeTransport._loop_reading()>.

It seems asyncio uses a completely different event loop implementation (by default) for windows and mac. Windows uses a Proactor event loop that uses I/O Completion Ports, while Unix uses the Selector Event loop based on the selector module.

It is worth noting that the selector event loop is compatible with BOTH Windows and Unix. except by default asyncio will use the proactor event loop on windows over the selector event loop.

The documentation also gives this snippet of code to manually configure the exact selector implementation to be used:

import asyncio
import selectors

class MyPolicy(asyncio.DefaultEventLoopPolicy):
   def new_event_loop(self):
      selector = selectors.SelectSelector()
      return asyncio.SelectorEventLoop(selector)

asyncio.set_event_loop_policy(MyPolicy())

I transferred this code into send.py. This got the following (new!) error:

PS C:\Users\Patrick\OneDrive - Clark University\Desktop\pydiode-main> pydiode --debug send 127.0.0.1 127.0.0.1
DEBUG:root:chunk_max_packets=100
DEBUG:root:chunk_duration=0.0073728
DEBUG:root:PACKET_HEADER.size=5
DEBUG:root:MAX_PAYLOAD=9211
DEBUG:root:chunk_max_data_bytes=921100
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Scripts\pydiode.exe\__main__.py", line 7, in <module>
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\site-packages\pydiode\pydiode.py", line 207, in main
    asyncio.run(async_main())
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\base_events.py", line 685, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\site-packages\pydiode\pydiode.py", line 125, in async_main
    await asyncio.gather(
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\site-packages\pydiode\send.py", line 111, in read_data
    data = await reader.read()
           ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\site-packages\pydiode\send.py", line 68, in read
    await **loop.connect_read_pipe**(lambda: protocol, sys.stdin.buffer)
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\base_events.py", line 1637, in connect_read_pipe
    transport = self._make_read_pipe_transport(pipe, protocol, waiter)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Patrick\AppData\Local\Programs\Python\Python312\Lib\asyncio\base_events.py", line 512, in _make_read_pipe_transport        
    raise NotImplementedError
NotImplementedError

This one I understand a lot more, than the previous one. It seems like in line with the compatibility documentation from my last comment, specifically this section
"Pipes are not supported, so the loop.connect_read_pipe() and loop.connect_write_pipe() methods are not implemented."

So, it seems like it is possible for the program to work with windows, HOWEVER, this would require us to make a version of the code that is built to be used with a proactor event loop (i think? if that seems reasonable).

(Apologies for the long winded comments)

@peterstory
Copy link
Contributor Author

The issue with selectors is that they have limited functionality on Windows:

Note The type of file objects supported depends on the platform: on Windows, sockets are supported, but not pipes, whereas on Unix, both are supported (some other types may be supported as well, such as fifos or special file devices).
https://docs.python.org/3/library/selectors.html#introduction

I think the error is triggered by the sender reading from STDIN:

class AsyncReader:
def __init__(self, chunk_max_data_bytes):
self.chunk_max_data_bytes = chunk_max_data_bytes
self.regular_file = stat.S_ISREG(os.fstat(sys.stdin.fileno()).st_mode)
self.stream_reader = None
async def read(self):
"""
Read data asynchronously from STDIN.
:returns: Bytes read from STDIN
"""
if self.regular_file:
# If connected to a regular file, there's no need for a
# StreamReader: we can quickly read the maximum amount of data for
# a chunk.
return await asyncio.get_event_loop().run_in_executor(
None, sys.stdin.buffer.read, self.chunk_max_data_bytes
)
else:
# If STDIN is a pipe or character device, data may become available
# incrementally. StreamReader lets us read the input incrementally,
# instead of waiting for a fixed number of bytes to become
# available.
# StreamReader doesn't work (and isn't necessary) with file
# redirection.
# https://stackoverflow.com/a/71627449
if not self.stream_reader:
loop = asyncio.get_event_loop()
self.stream_reader = asyncio.StreamReader()
protocol = asyncio.StreamReaderProtocol(self.stream_reader)
await loop.connect_read_pipe(lambda: protocol, sys.stdin.buffer)
# Read up to the maximum amount of data in a chunk
return await self.stream_reader.read(self.chunk_max_data_bytes)

So perhaps the solution is to add some extra code to AsyncReader to handle reading from STDIN on Windows. Of course, we should confirm that the issue is indeed caused by this part of the code. To test this, I recommend creating a simple test program based on AsyncReader. For example, you could try printing STDIN to STDOUT using AsyncReader, and see if you get the same error.

@Patrick-Ziegler
Copy link

I don't know if you saw the edits for my previous note (I had not seen your reply and figured id just edit instead of making a new note). but it basically just confirmed what your note said.

I havent actually tested it yet with AsyncReader, but that seems like it is probably it (and seems much simpler than my idea of making a completely different file for windows users). I found this thread, which I know is coded in C but it seems that STDIN and IOCP do not like each other very much.

I will get back to you when I test this a little more thoroughly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants