allenai / ai2thor

An open-source platform for Visual AI.
http://ai2thor.allenai.org
Apache License 2.0
1.15k stars 216 forks source link

Enable running on Windows #1192

Open garfieldnate opened 6 months ago

garfieldnate commented 6 months ago

Add Windows to ExternalBuild so that builds on Windows are recognized and used when appropriate.

Fix WsgiServer, which is used for communicating with the Unity process on Windows, by pinning to an older version of flask/werkzeug that still supports TCP keepalive. Some background:

AgentManager.cs creates a single Socket object and communicates through it for the lifetime of the application. This is different from the default HTTP request flow, where a TCP connection is created at the beginning of the request and destroyed at the end. For this connection to work properly, TCP keepalive must be enabled.

Previously, setting werkzeug.serving.WSGIRequestHandler. protocol_version = "HTTP/1.1" (as WsgiServer does) would activate TCP keepalive by default. However, the flask/werkzeug built-in server relies on the python built-in server, which had (has?) a bug in keepalive connections: if you didn't read the request body, it wouldn't properly empty the socket, and then the next time an HTTP request handler fired the handler would read data from the previous request. Because the server shipped with flask/werkzeug is not meant to be productionizable, and several users were complaining about this issue, the devs just disabled keepalive altogether and moved on. Removal note here: https://werkzeug.palletsprojects.com/en/2.3.x/changes/#version-2-1-2. More details here: https://github.com/pallets/werkzeug/issues/2397.

I think WsgiServer was implemented with a previous version of flask/ werkzeug, and it broke when the version numbers were updated.

I understand that Windows is not officially supported, but its usage is a requirement for some, and even just enabling users to build on Windows themselves will be very useful.

These changes do not affect the FifoServer, which is used on MacOS and Linux.

garfieldnate commented 6 months ago

@Lucaweihs I pushed the other needed changes. It's ready for review :) .