implement support for the repl goal w/ the daemon
#3774
Comments
|
For the |
|
I believe that I have a mostly-working implementation of code to read from the server's nailgun socket and write to stdin of the child process... I think there is one more tweak necessary though, as the child JVM process is exiting immediately as if stdin were empty/closed. I'm also seeing in master that there is a failure where (seemingly) raw data (without a chunk header) is being written to the socket. This could potentially be a concurrency issue (if |
|
The issue I'm seeing in master is that a blob of stdout data without a header is being placed on the socket: ... I can't locate the writer of the value on the server, even tracing aggressively using At this point, I can only assume that this is a concurrency issue due to the use of forked processes writing to those file-like objects in a thread-unsafe way. There are two things that don't quite align with this assumption though:
Lacking other ways to make forward process, I'm considering adjusting NailgunStreamWriter to write to a |
|
@kwlzn pointed to pants/src/python/pants/java/nailgun_io.py Lines 114 to 115 in e3a2276 Proceeding with the pipe approach. |
### Problem The `DaemonPantsRunner` did not support consuming `STDIN` and `STDIN_EOF` chunks off of the nailgun socket, and so subprocesses of the pants daemon that read from `stdin` would immediately exit. Additionally, because `stdin`/`stderr` were not actual pipes in the `DaemonPantsRunner` (rather, they were file-like objects that wrote directly to the nailgun socket) we were not able to fork and still proxy `stdout`/`stderr` from the forked process. ### Solution * Replace the synchronous file-like `NailgunStreamWriter` with an adjusted version of the previous reader code that uses a thread to copy from an anonymous `os.pipe`/`os.openpty` onto the socket. * Use `NailgunStreamWriter` to publish `sys.stdout`/`sys.stderr` from the server, and `sys.stdin` from the client. * Implement `NailgunStreamStdinReader` to read `stdin` nailgun chunks on the server and proxy them to `sys.stdin`. * Explicitly pass `sys.stdin` and `sys.stderr` when forking with SubprocessExecutor to disable additional handling in `Popen` that caused forked processes to write directly to fd 1 (the socket, in this case). * Add an `@ensure_daemon` decorator, and use it to ensure that python and scala repls work in the context of the daemon. * Move the watchman directory under the `subprocess_dir`, to fix an issue where calling `./pants kill-pantsd` would fail if the `subprocess_dir` matched but the `workdir` didn't. ### Result Fixes #3774. There is one slight oddity, which is that our `Ctrl+D` handling doesn't quite align with daemonless usage: opened #5058 about that.
### Problem The `DaemonPantsRunner` did not support consuming `STDIN` and `STDIN_EOF` chunks off of the nailgun socket, and so subprocesses of the pants daemon that read from `stdin` would immediately exit. Additionally, because `stdin`/`stderr` were not actual pipes in the `DaemonPantsRunner` (rather, they were file-like objects that wrote directly to the nailgun socket) we were not able to fork and still proxy `stdout`/`stderr` from the forked process. ### Solution * Replace the synchronous file-like `NailgunStreamWriter` with an adjusted version of the previous reader code that uses a thread to copy from an anonymous `os.pipe`/`os.openpty` onto the socket. * Use `NailgunStreamWriter` to publish `sys.stdout`/`sys.stderr` from the server, and `sys.stdin` from the client. * Implement `NailgunStreamStdinReader` to read `stdin` nailgun chunks on the server and proxy them to `sys.stdin`. * Explicitly pass `sys.stdin` and `sys.stderr` when forking with SubprocessExecutor to disable additional handling in `Popen` that caused forked processes to write directly to fd 1 (the socket, in this case). * Add an `@ensure_daemon` decorator, and use it to ensure that python and scala repls work in the context of the daemon. * Move the watchman directory under the `subprocess_dir`, to fix an issue where calling `./pants kill-pantsd` would fail if the `subprocess_dir` matched but the `workdir` didn't. ### Result Fixes pantsbuild#3774. There is one slight oddity, which is that our `Ctrl+D` handling doesn't quite align with daemonless usage: opened pantsbuild#5058 about that.
currently,
replis known to not work with the daemon due to an unimplemented bi-directional user input path here:pants/src/python/pants/bin/daemon_pants_runner.py
Lines 99 to 100 in e3a2276
in order to get users using the daemon more generally, we'll need to get this implemented.
The text was updated successfully, but these errors were encountered: