- After commit bb428520, there has been the disadvantage:
7) Pseudo console cannot be activated if it is already activated for
another process on same pty.
This patch clears this disadvantage.
- After commit bb428520, there has been the disadvantage:
2) The apps which use console API cannot be debugged with gdb. This
is because pseudo console is not activated since gdb uses
CreateProcess() rather than exec(). Even with this limitation,
attaching gdb to native app, in which pseudo console is already
activated, works.
This patch clears this disadvantage.
- PTY has a problem that the key input, which is typed during windows
native app is running, disappears when it returns to shell. This is
beacuse pty has two input pipes, one is for cygwin apps and the other
one is for native windows apps. The key input during windows native
program is running is sent to the second input pipe while cygwin
shell reads input from the first input pipe. This issue had been
fixed once by commit 29431fcb, however, the new implementation of
pseudo console support by commit bb428520 could not inherit this
feature. This patch realize transfering input data between these
two pipes bidirectionally by utilizing cygwin-console-helper process.
The helper process is launched prior to starting the non-cygwin app,
however, exits immediately unlike previous implementation.
The isdev_dev check in rmdir is unclean. Create a virtual method
fhandler_dev::rmdir to handle this transparently.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- The functions pty_master_thread() and pty_master_fwd_thread()
should be static (i.e. should not access class member) because
the instance is deleted if the master is dup()'ed and the first
master is closed. In this case, because the dup()'ed instance
still exists, these master threads are also still alive even
though the instance has been deleted. As a result, accesing
class members in these functions causes accessi violation.
Addresses:
https://cygwin.com/pipermail/cygwin-developers/2021-January/012030.html
- After commit 232fde0e, pty changes console code page when the first
non-cygwin app is executed. If pty is started in real console device,
pty changes the code page of root console. This causes very annoying
result because changing code page changes the font of command prompt
if console is in legacy mode. This patch avoids this by creating a
new invisible console for the first pty started in console device.
- The function close_pseudoconsole() should be static so that it
can be safely called in spawn.cc even after the fhandler_pty_slave
instance has been deleted. That is, there is a problem with the
current code. This patch fixes the issue.
- If application changes the console mode, mode management introduced
by commit 10d8c278 will be corrupted. For example, stdout of jansi
v2.0.1 or later is piped to less, jansi resets the xterm mode flag
ENABLE_VIRTUAL_TERMINA_PROCESSING when jansi is terminated. This
causes garbled output in less because less needs this flag enabled.
This patch fixes the issue.
poll has code that checks whether a socket descriptor has a failed
connect or whether shutdown has been called. This previously worked
only for wsock sockets. Tweak the code so that it applies to all
sockets.
To help with this, add a virtual saw_shutdown_read method to the
fhandler_socket class, and define a version of it for AF_UNIX sockets.
A version for wsock sockets already existed.
Untested.
When a process sends a file descriptor via an SCM_RIGHTS control
message, it creates a temporary copy of the fhandler associated with
that descriptor and sends a serialization of that copy. The
deserialization done by the receiver involves duplicating handles from
the copy, so the latter must stay alive until the deserialization is
done. But it must ultimately be closed in order to avoid a memory
leak.
We coordinate all this as follows:
- Introduce a new struct scm_pending_fd that contains information
about the temporary copy. For brevity, call such a struct a
"pending fd" in what follows.
- Maintain a list of pending fds in shared memory.
- Add several methods for manipulating the list to the af_unix_shmem_t
and fhandler_socket_unix classes.
- Also add a lock, 'scm_fd_lock', to control access to the list.
- When a serialized fhandler is received, the receiver sends an ack
back to the sender in an administrative packet with a control
message of a new (Cygwin-specific) type SCM_RIGHTS_ACK.
- grab_admin_pkt is called in various places to process these packets.
A complication here is that the process that calls grab_admin_pkt
might not be the process that originally sent the serialized
fhandler. (It could be a subprocess of the original process, for
example.) This is why we need to maintain the list of pending fds
in shared memory.
- Each fhandler_socket_unix keeps a count of the pending fds that it
has created but not yet processed; this count is in a new data
member 'my_npending_fd'.
- fhandler_socket_unix::close tries to process any remaining pending
fds before closing, but it gives up after a short timeout and
forcibly deletes them if necessary.
Make them member functions of the fhandler_socket_unix class.
Make them use void * instead of fh_ser * so that fhandler.h doesn't
need to know about fh_ser.
This allows duplication of handles from an fhandler created in a
different process. For now, this is implemented only for
fhandler_base and fhandler_disk_file.
Override this in each derived class to give the size of each fhandler
class so that the size can be computed dynamically from an
fhandler_base pointer.
Replace the 'WCHAR pipe_name_buf[48]' class member by 'PWCHAR
pipe_name_buf', and allocate space for the latter as needed.
Change the default constructor to accommodate this change, and add a
destructor that frees the allocated space.
Also change get_pipe_name and clone to accommodate this change.
Add a new HANDLE argument to peek_pipe and peek_pipe_poll so that the
caller can specify a pipe handle to use in lieu of get_handle(). Use
this in recvmsg to make the MSG_PEEK flag work for unbound datagram
sockets.
Untested.
Define it to be UINT16_MAX, since the pckt_len member of
af_unix_pkt_hdr_t is of type uint16_t.
Previously it was 65536 (== UINT16_MAX + 1), which is 0 when cast to
uint16_t. So an attempt to write a packet of size MAX_AF_PKT_LEN
actually wrote 0 bytes.
And use it in recvmsg.
I'm not sure this implementation is what was intended when the
evaluate_cmsg_data method was added.
For now, just support an ancillary data block consisting of a single
cmsghdr, containing SCM_CREDENTIALS.
For convenience, add a 'mshdr *' argument and make the 'cloexec'
argument false by default. The 'cloexec' argument is not currently
used, and I want to avoid having to artificially specify a value for
it when recvmsg calls evaluate_cmsg_data.
select.cc:set_bits sets the readfds entry on wsock sockets that have a
failed connect. Tweak the code so that it applies to all sockets.
To help with this, add virtual connect_state methods to the
fhandler_socket class. Versions of these methods for
fhandler_socket_wsock and fhandler_socket_unix already exist.
Extract from grab_admin_pkg two new methods, record_shut_info and
process_admin_pkg. Also add a new 'peek' argument to grab_admin_pkg.
If this is true, peek to see if the next packet in the pipe is an
administrative packet. Otherwise, assume we already know it is.
When reading from a stream socket, it's possible that the caller
requested fewer bytes that are available in the packet. In this case
there is user data left in the pipe without a packet header.
Add a member '_unread' to the af_unix_shmem_t class to keep track of
this, and add corresponding methods 'get_unread' and 'set_unread' to
af_unix_shmem_t and fhandler_socket_unix.
Both flags are outdated and collide with official flags in
sys/_default_fcntl.h, which may result in weird misbehaviour
of file functions.
O_NOSYMLINK is not used anyway.
O_DIROPEN is used in fhandler_virtual and derived classes.
The collision with O_NOFOLLOW results in spurious EISDIR
errors when, e. g., reading files in the registry.
fhandler_base::open_fs uses O_DIROPEN in the call to
fhandler_base::open, but it's not used in this context
further down the road.
Drop both flags and create an alternative "diropen" bool
flag in fhandler_virtual.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- Pseudo console internally sends escape sequence CSI6n (query cursor
position) on startup of non-cygwin apps. If the terminal does not
support CSI6n, CreateProcess() hangs waiting for response. To prevent
hang, this patch disables pseudo console if the terminal does not
have CSI6n. This is checked on the first execution of non-cygwin
app using the following steps.
1) Check if the terminal support ANSI escape sequences by looking
into terminfo database. If terminfo has cursor_home (ESC [H),
the terminal is supposed to support ANSI escape sequences.
2) If the terminal supports ANSI escape sequneces, send CSI6n for
a test and wait for a responce for 40ms.
3) If there is a responce within 40ms, CSI6n is supposed to be
supported.
Also set-title capability is checked, and removes escape sequence
for setting window title if the terminal does not have the set-
title capability.
- In this implementation, pseudo console is created for each native
console app. Advantages and disadvantages of this implementation
over the previous implementation are as follows.
Advantages:
1) No performance degradation in pty output for cygwin process.
https://cygwin.com/pipermail/cygwin/2020-February/243858.html
2) Free from the problem caused by difference of behaviour of control
sequences between real terminal and pseudo console.
https://cygwin.com/pipermail/cygwin/2019-December/243281.htmlhttps://cygwin.com/pipermail/cygwin/2020-February/243855.html
3) Free from the problem in cgdb and emacs gud.
https://cygwin.com/pipermail/cygwin/2020-January/243601.htmlhttps://cygwin.com/pipermail/cygwin/2020-March/244146.html
4) Redrawing screen on executing native console apps is not necessary.
5) cygwin-console-helper is not necessary for the pseudo console
support.
6) The codes for pseudo console support are much simpler than that
of the previous one.
Disadvantages:
1) The cygwin program which calls console API directly does not work.
2) The apps which use console API cannot be debugged with gdb. This
is because pseudo console is not activated since gdb uses
CreateProcess() rather than exec(). Even with this limitation,
attaching gdb to native apps, in which pseudo console is already
activated, works.
3) Typeahead key inputs are discarded while native console app is
executed. Simirally, typeahead key inputs while cygwin app is
executed are not inherited to native console app.
4) Code page cannot be changed by chcp.com. Acctually, chcp works
itself and changes code page of its own pseudo console. However,
since pseudo console is recreated for another process, it cannot
inherit the code page.
5) system_printf() does not work after stderr is closed. (Same with
cygwin 3.0.7)
6) Startup time of native console apps is about 3 times slower than
previous implemenation.
7) Pseudo console cannot be activated if it is already activated for
another process on same pty.
The fifo_reader thread function and the function select.cc:peek_fifo()
can both change the state of a fifo_client_handler. These changes are
made under fifo_client_lock, so there is no race, but the changes can
still be incompatible.
Add code to make sure that only one of these functions can change the
state from its initial fc_listening state. Whichever function does
this calls the fhandler_fifo::record_connection method, which is now
public so that peek_fifo can call it.
Slightly modify that method to make it suitable for being called by
peek_fifo.
Make a few other small changes to the fifo_reader thread function to
change how it deals with the STATUS_PIPE_CLOSING value that can
(rarely) be returned by NtFsControlFile.
Add commentary to fhandler_fifo.cc to explain fifo_client connect
states and where they can be changed.
Don't try to read from fifo_client_handlers that are in the fc_closing
state. Experiments have shown that this always yields
STATUS_PIPE_BROKEN, so it just wastes a Windows system call.
Re-order the values in enum fifo_client_connect_state to reflect the
new status of fc_closing.
Rename the existing set_state() to query_and_set_state() to reflect
what it really does. (It queries the O/S for the pipe state.) Add a
new set_state() method, which is a standard setter, and a
corresponding getter get_state().
fhandler_fifo::take_ownership() is called from select.cc::peek_fifo
and fhandler_fifo::raw_read and could potentially block indefinitely
if something goes wrong. This is always undesirable in peek_fifo, and
it is undesirable in a nonblocking read. Fix this by adding a timeout
parameter to take_ownership.
Arbitrarily use a 1 ms timeout in peek_fifo and a 10 ms timeout in
raw_read. These numbers may have to be tweaked based on experience.
Replace the call to cygwait in take_ownership by a call to WFSO.
There's no need to allow interruption now that we have a timeout.
Use cygwait in take_ownership to allow interruption while waiting to
become owner. Return the cygwait return value or a suitable value to
indicate an error.
raw_read now checks the return value and acts accordingly.
Add a bool member 'last_read' to the fifo_client_handler structure,
which is set to true on a successful read. This is used by raw_read
as follows.
When raw_read is called, it first locates the writer (if any) for
which last_read is true. raw_read tries to read from that writer and
returns if there is input available. Otherwise, it proceeds to poll
all the writers, as before.
The effect of this is that if a writer writes some data that is only
partially read, the next attempt to read will continue to read from
the same writer. This should reduce the interleaving of output from
different writers.
When a reader opens, it needs to block if there are no writers open
(unless is is opened with O_NONBLOCK). This is easy for the first
reader to test, since it can just wait for a writer to signal that it
is open (via the write_ready event). But when a second reader wants
to open, all writers might have closed.
To check this, use a new '_nwriters' member of struct fifo_shmem_t,
which keeps track of the number of open writers. This should be more
reliable than the previous method.
Add nwriters_lock to control access to shmem->_nwriters, and remove
reader_opening_lock, which is no longer needed.
Previously only readers had access to the shared memory, but now
writers access it too so that they can increment _nwriters during
open/dup/fork/exec and decrement it during close.
Add an optional 'only_open' argument to create_shmem for use by
writers, which only open the shared memory rather than first trying to
create it. Since writers don't need to access the shared memory until
they have successfully connected to a pipe instance, they can safely
assume that a reader has already created the shared memory.
For debugging purposes, change create_shmem to return 1 instead of 0
when a reader successfully opens the shared memory after finding that
it had already been created.
Remove check_write_ready_evt, write_ready_ok_evt, and
check_write_ready(), which are no longer needed.
When opening a writer and looping to try to get a connection, recheck
read_ready at the top of the loop since the number of readers might
have changed.
To slightly speed up the process of opening the first reader, take
ownership immediately rather than waiting for the fifo_reader_thread
to handle it.
When the owning reader closes and there are still readers open, the
owner needs to wait for a new owner to be found before closing its
fifo_client handlers. This involves a loop in which dec_nreaders is
called at the beginning and inc_nreaders is called at the end. Any
other reader that tries to access shmem->_nreaders during this loop
will therefore get an inaccurate answer.
Fix this by adding an nreaders method and using it instead of
dec_nreaders and inc_nreaders. Also add nreaders_lock to control
access to the shmem->_nreaders.
Make various other changes to improve the reliability of finding a new
owner.
Use WSAIoctl(SIO_KEEPALIVE_VALS) on older systems.
Make sure that keep-alive timeout is equivalent to
TCP_KEEPIDLE + TCP_KEEPCNT * TCP_KEEPINTVL on older systems,
even with TCP_KEEPCNT being a fixed value on those systems.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- Commit c4b060e3fe3bed05b3a69ccbcc20993ad85e163d seems to be not
enough. Moreover, it does not work as expected at all in Win10
1809. This patch essentially reverts that commit and add another
fix. After all, the cause of the problem was a race issue in
switch_to_pcon_out flag. That is, this flag is set when native
app starts, however, it is delayed by wait_pcon_fwd(). Since the
flag is not set yet when less starts, the data which should go
into the output_handle accidentally goes into output_handle_cyg.
This patch fixes the problem more essentially for the cause of
the problem than previous one.
- After commit 29431fcb5b14d4c5ac3b3161a076eb1a208349d9, the issue
reported in https://cygwin.com/pipermail/cygwin/2020-May/245057.html
occurs. This is caused by the following mechanism. Cygwin less
called from non-cygwin git is executed under /dev/cons* rather
than /dev/pty* because parent git process only inherits pseudo
console handle. Therefore, less sets ICANON flag for /dev/cons*
rather than original /dev/pty*. When pty is switched to non-cygwin
git process, line_edit() is used in fhandler_pty_master::write()
only to set input_available_event and read ahead buffer is supposed
to be flushed in accept_input(). However, ICANON flag is not set
for /dev/pty*, so accept_input() is not called unless newline
is entered. As a result, the input data remains in the read ahead
buffer. This patch fixes the issue.
- After commit 774b8996d1f3e535e8267be4eb8e751d756c2cec, cursor
keys do not work in vim under ConEmu without cygwin-connector.
This patch fixes the issue.
This reverts commit 39a9cd94651d306117c47ea1ac3eab45f6098d0e.
There is no need to explicitly take ownership in fixup_after_exec; if
ownership transfer is needed, it will be taken care of by
fhandler_fifo::close when the parent closes. Moreover, closing the
parent's fifo_reader_thread can cause problems, such as the one
reported here:
https://cygwin.com/pipermail/cygwin-patches/2020q2/010235.html
- After commit 071b8e0cbd4be33449c12bb0d58f514ed8ef893c, the problem
reported in https://cygwin.com/pipermail/cygwin/2020-May/244873.html
occurs. This is due to freeing console device accidentally rather
than pseudo console. This patch makes sure to call FreeConsole()
only if the process is attached to the pseudo console of the current
pty.
There are currently three functions that call NtQueryInformationFile
to determine the state of a pipe instance. Do this only once, in a
new fifo_client_handler::set_state () function, and call that when
state information is needed.
Remove the fifo_client_handler methods pipe_state and get_state, which
are no longer needed.
Make fhandler_fifo::get_fc_handler return a reference, for use in
select.cc:peek_fifo.
Make other small changes to ensure that this commit doesn't change any
decisions based on the state of a fifo_client_handler.
The tricky part is interpreting FILE_PIPE_CLOSING_STATE, which we
translate to fc_closing. Our current interpretation, which is not
changing as a result of this commit, is that the writer at the other
end of the pipe instance is viewed as still connected from the point
of view of raw_read and determining EOF.
But it is not viewed as still connected if we are deciding whether to
unblock a new reader that is trying to open.
fifo_reader_id_t::operator == and != have been defined without type
accidentally. For some weird reason, only x86 gcc complains about
this problem, not x86_64 gcc.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
