Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/118#discussion_r146113065
--- Diff: src/protocols/ssh/client.c ---
@@ -70,8 +70,14 @@ int guac_ssh_client_free_handler(guac_client* client) {
/* Free terminal (which may still be using term_channel) */
if (ssh_client->term != NULL) {
- guac_terminal_free(ssh_client->term);
+ /* Close user input pipe to stop reading in ssh_input_thread */
+ close(ssh_client->term->stdin_pipe_fd[1]);
+ close(ssh_client->term->stdin_pipe_fd[0]);
+ ssh_client->term->stdin_pipe_fd[1] = -1;
+ ssh_client->term->stdin_pipe_fd[0] = -1;
+
--- End diff --
Well:
1. Keep in mind that guacamole-server is meant to be as POSIX-compatible as
possible. Unless something cannot be done without platform-specific code
(`move-fd.c` is the only current case of this in the codebase), then it needs
to be done without platform-specific code.
2. You can't actually send EOF, as EOF isn't data, but a state. If you want
reads to fail from one end of the pipe and return EOF, the way to do that is to
close the file descriptor on the other end.
Closing the file descriptors to force reads to fail is fine. The problem is
requiring the caller to know that this is what is happening. As long as the
function naming and documentation do not require the caller to have such
internal knowledge of `guac_terminal`, then things are OK.
It makes sense for the caller to need to know:
* Terminal reads/writes may block (I believe this is already documented).
* Some specific function (`guac_terminal_stop()`? `guac_terminal_close()`?)
can be used to forcibly unblock any pending reads/writes and cease all terminal
I/O.
It doesn't make sense for the caller to need to know:
* `guac_terminal` contains multiple file descriptors for pipes which are
used internally by the read/write functions and should be closed to force those
reads/writes to unblock and fail.
---
