This reverts commit 3d8f98c395.
In addition to the issues mentioned on the GitHub page for this commit,
it also broke the CentOS 7 build.
Note one can locally test the CentOS 7 build via:
./docker/docker_run_tests.sh ./docker/centos7.Dockerfile
Be more careful with sign extension issues stemming from the differences in how
an untyped literal is promoted to an integer vs how a typed (and signed) `char`
is promoted to an integer.
Also convert some `const[expr] static xxx` to `const[expr] xxx` where it makes
sense to let the compiler deduce on its own whether or not to allocate storage
for a constant variable rather than imposing our view that it should have STATIC
storage set aside for it.
A few call sites were not making use of the `XXX_LEN` definitions and were
calling `strlen(XXX)` - these have been updated to use `const_strlen(XXX)`
instead.
I'm not sure if any toolchains will have raise any issues with these changes...
CI will tell!
Let's hope this doesn't causes build failures for e.g. musl: I just
know it's good on macOS and our Linux CI.
It's been a long time.
One fix this brings, is I discovered we #include assert.h or cassert
in a lot of places. If those ever happen to be in a file that doesn't
include common.h, or we are before common.h gets included, we're
unawaringly working with the system 'assert' macro again, which
may get disabled for debug builds or at least has different
behavior on crash. We undef 'assert' and redefine it in common.h.
Those were all eliminated, except in one catch-22 spot for
maybe.h: it can't include common.h. A fix might be to
make a fish_assert.h that *usually* common.h exports.
fd_monitor is used when an external command pipes into a buffer, e.g. for
command substitutions. It monitors the read end of the external command's
pipe in the background, and fills the buffer as data arrives. fd_monitor is
multiplexed, so multiple buffers can be monitored at once by a single
thread.
It may happen that there's no active buffer fill; in this case fd_monitor
wants to keep its thread alive for a little bit in case a new one arrives.
This is useful for e.g. handling loops where you run the same command
multiple times.
However there was a bug due to a refactoring which caused fd_monitor to
exit too aggressively. This didn't affect correctness but it meant more
thread creation and teardown.
Fix this; this improves the aliases.fish benchmark by about 20 msec.
No need to changelog this IMO.
Cygwin tests are failing because cygwin has a low limit of only 64 fds in
select(). Extend select_wrapper_t to also support using poll(), according to
a FISH_USE_POLL new define. All systems now use poll() except for Mac.
Rename select_wrapper_t to fd_readable_set_t since now it may not wrap
select().
This allows the deep-cmdsub.fish test to pass on Cygwin.
clang-tidy wrongly sees an std::move to a const ref parameter and
believes it to be pointless. The copy constructor however is deleted.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
select_wrapper_t wraps up the annoying bits of using select(): keeping
track of the max fd, passing null for boring parameters, and
constructing the timeout. Introduce a wrapper struct for this and
replace the existing uses of select() with the wrapper.
fd_monitor_t allows observing a collection of fds. It also has its own
fd, which it uses to awaken itself when there are changes. Switch to
using fd_event_signaller_t instead of a pipe; this reduces the number of
file descriptors and is more efficient under Linux.
This concerns how fish prevents its own fds from interfering with
user-defined fd redirections, like `echo hi >&5`. fish has historically
done this by tracking all user defined redirections when running a job,
and ensuring that pipes are not assigned the same fds. However this is
annoying to pass around - it means that we have to thread user-defined
redirections into pipe creation.
Take a page from zsh and just ensure that all pipes we create have fds in
the "high range," which here means at least 10. The primary way to do this
is via the F_DUPFD_CLOEXEC syscall, which also sets CLOEXEC, so we aren't
invoking additional syscalls in the common case. This will free us from
having to track which fds are in user-defined redirections.
In preparation for fixing #7559, add a function poke_item to fd_monitor.
fd_monitor has a list of file descriptors, and invokes a callback when an
fd becomes readable. With this change, we assign each item a unique ID and
return it when the item is added; the ID may then be used to invoke the
callback explicitly.
The idea is that we can stop reading from the pipe associated with the
cmdsub when the job is finished, even if the pipe is still open.
fd_monitor is a new class which can monitor a set of fds, waiting for them
to become readable. When an fd becomes readable, a callback is invoked.
Timeouts are also supported.
This is intended to replace the "bufferfill" threads. Rather than one
thread per bufferfill, we will have a single fd_monitor which can service
multiple bufferfills. This helps today with nested command substitutions,
and will help in the future with concurrent execution.
