ClarkuCSCI / pydiode

Transfer data through a unidirectional network (i.e., a data diode)
MIT License
3 stars 1 forks source link

Add Support for Windows #6

Open peterstory opened 11 months ago

peterstory commented 11 months ago

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

Patrick-Ziegler commented 6 months ago

From my testing on windows I found the following:

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](https://docs.python.org/3.12/library/asyncio-platforms.html#all-platforms)

Additionally here is an old but [seemingly related github ticket](https://github.com/python/cpython/issues/71019). 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,](https://stackoverflow.com/questions/62412754/python-asyncio-errors-oserror-winerror-6-the-handle-is-invalid-and-runtim) 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 commented 6 months ago

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 commented 6 months ago

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: https://github.com/ClarkuCSCI/pydiode/blob/0619a192455e939d2517ab15d55b921308c803d9/src/pydiode/send.py#L29-L62

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 commented 6 months ago

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