redirecting >&4 sends data to iothread_service_completion()

[floam]

Should this happen?

~ > echo foo >&4
~ > Unknown wakeup byte 66 in iothread_service_completion
Unknown wakeup byte 6f in iothread_service_completion
Unknown wakeup byte 6f in iothread_service_completion
Unknown wakeup byte 0a in iothread_service_completion

if not interactive, does what other fds do:

An error occurred while redirecting file descriptor 1
dup2: Bad file descriptor

[krader1961]

See iothread_init() in src/iothread.cpp. Since fish will normally be started with only fd 0, 1, and 2 open that function will open fd 3 and 4 on a pipe for communication with the background I/O threads. As you can see from that code you can actually interfere with its operation, albeit apparently without harm, by doing echo -n cd >&4 since the two magic values that are valid in that pipe are 99 (c) and 100 (d).

Having said all that what did you expect to happen? Normally if you redirect to an invalid file descriptor you'll get an error:

$ echo hello >&88
An error occurred while redirecting file descriptor 1
dup2: Bad file descriptor

The redirection code should probably be modified to explicitly test for the use of file descriptors used internally and emit the same error since doing such a redirection is nonsensical. We should also remove the dup2 error message or change it to a debug message.

[floam]

Having said all that what did you expect to happen?

An error.

It seemed worrying that I could get that function to eat my 99/100s and seemingly mess with fish internally - although I can't get anything terrible to happen.

Redirecting fd 3 will similarly send characters to the tty while disrupting completion behavior.

echo -n this normally doesn\'t work >&4 & cat <&3 &

Also, 4 different errors for each of: echo foo >&{4, 5, 6, 7} (actually, no error with 6).

Maybe if these were simply given higher fd numbers nobody would be as likely to encounter them accidentally.

[krader1961]

For anyone inclined to fix this I suggest adding a new class that tracks file descriptors used privately by fish. It should have methods such as "make_fd_private", "make_fd_public", and "is_private". Those method names are merely suggestions so you are welcome to use other names. The behavior of each method should be obvious from the suggested name.

The class would be instantiated using the singleton pattern; i.e., a single global instance of the class. Functions like iothread_init() and the code in src/exec.cpp would then call those methods for the file descriptors they create and destroy for private use. The file descriptor redirection logic would then call the is_private() method on the singleton to decide if the fd can be used as a source or target of the redirection.

Also, any existing code, such as the iothread_init() function, should be modified to use dup2() to reassign their file descriptors to a range starting at 254 and counting down. That's because the vast majority of scripts use idioms like

exec 3>/tmp/file
echo hello >&3

Such scripts almost always use low numbered file descriptors. That pattern isn't currently legal in fish but should be. As part of fixing this issue we should make it easier to support that idiom. So there should also be a function added that takes a file descriptor returned by something like the pipe() function and transforms it into the highest unused file descriptor less than or equal to 254.

[ridiculousfish]

We do this in eval:

echo "begin; $argv "\n" ;end <&3 3<&-" | source 3<&0

Rather than a lot of machinery to track file descriptors (which would duplicate a lot of the CLOEXEC logic) I would rather we always just fork for processes that have redirections of file descriptors larger than 2. We already do some of this for output (look for must_fork in exec.cpp). That is, if you do this:

echo foo >&4

we'll run the builtin, get the buffered output, fork a new process, and write it to fd 4. This might error, since fd 4 won't be open in the child - but it might succeed if we have a stdin redirection in a pipe, i.e.:

echo foo >&4 | cat 0<&4

might plausibly work.

[krader1961]

We do this in eval: echo "begin; $argv "\n" ;end <&3 3<&-" | source 3<&0,

Doesn't the fact that command requires a lengthy comment in the source of the eval function to discuss the internal, private, details of fish's implementation worry you? To me it says we do not want to codify that pattern. Quite the contrary. That behavior is far too subtle for a "friendly" shell.

we'll run the builtin, get the buffered output, fork a new process, and write it to fd 4. This might error, since fd 4 won't be open in the child...

That seems rather expensive. Which isn't a big deal today since we only support fd redirection in that manner in a

[floam]

Rather than a lot of machinery to track file descriptors (which would duplicate a lot of the CLOEXEC logic) I would rather we always just fork for processes that have redirections of file descriptors larger than 2. We already do some of this for output (look for must_fork in exec.cpp). That is, if you do this:
echo foo >&4
we'll run the builtin, get the buffered output, fork a new process, and write it to fd 4. This might error, since fd 4 won't be open in the child - but it might succeed if we have a stdin redirection in a pipe, i.e.:
echo foo >&4 | cat 0<&4
might plausibly work.

I've got a thousand yard stare after reading through eval.fish and then trying to figure out what we're saying here. There's some spooky, hard stuff here! Is this an example of how that works and how we want it to work, or illustrating something I didn't quite catch?

If forking gets us closer to the kind of usage @krader1961 mentioned above which fish is worse off for lacking lacking IMO (exec 3>/tmp/file; echo hello >&3) - then that is cool - and spooky.

[ridiculousfish]

Doesn't the fact that command requires a lengthy comment in the source of the eval function to discuss the internal, private, details of fish's implementation worry you? To me it says we do not want to codify that pattern. Quite the contrary. That behavior is far too subtle for a "friendly" shell.

Completely agree - this is clearly a sh holdover.

AFAICT the only legitimate use for redirections above 2 is to "save and restore" an existing file descriptor, or sometimes swap them, and this is always extremely confusing and obscure. Everyone would be better off if there were an alternative readable way to accomplish this, and then I think we could disallow redirections above 2 altogether (which would be a nice simplification in fish).

However until we have that, we have to keep our existing uses (eval and something in __fish_print_help that I don't understand) working.

[floam]

AFAICT the only legitimate use for redirections above 2 is to "save and restore" an existing file descriptor, or sometimes swap them

Surely there are reasons to want more channels of output. I do this in C:

void main(int argc, char* argv[]) {
    dprintf(1, "Regular output");
    dprintf(15, "This is debug output that won't print anything unless you 15>&1 me\n");
    dprintf(16, "This is more deeper debuggy\n");
}

With obvious goals and pleasant results. Maybe it's a bad pattern?

[ridiculousfish]

Fascinating, that's the first I've ever heard of someone doing that.

[floam]

Really though - it actually feels easier in C there than a shell. One thing is the lack of syntax - but what's the technical reason why a fd can't be immediately valid for use? It's probably obvious, but I'd rather my shell let me:

function output
   echo foo >&3
   echo bar
end

> output
bar
> output 3>&1
foo
bar

instead of:

Error while writing to stdout
write_loop: Bad file descriptor
bar

I'm sure there's some good reason - doesn't work in any shell I've tried. But it's easy if it's another process.

int main(int argc, char* argv[])
{
    dprintf(10, "foo");
    dprintf(1, "bar");
}

> ./a.out 
bar
> ./a.out 3>&1
foobar

[floam]

Err, spoke too soon. causes an error in bash & zsh when not an external script, but works in ksh93u+ 2012-08-01:

$ function foo { echo foo; echo bar >&3; }
$ foo 3>&1                      
foo
bar
$ foo
foo
$

[krader1961]

Redirecting fds greater than two is actually quite useful. In other shells you can do things like

exec 3>/some/file
echo abc >&3
echo def >&3
exec 3>-

Now obviously that's a silly example that doesn't really need fd 3. A more realistic example would involve using a named pipe, where you don't want it being opened and closed multiple times because that would confuse the process reading from the pipe. Or a more complicated situation, possibly involving a loop, where you want to write a lot to a destination without paying the overhead of opening and closing it with each I/O. Of course, at the moment fish doesn't support using exec in that fashion but I think we should.

[kivikakk]

Redirecting fds greater than two is actually quite useful

This pattern is used quite frequently — try searching for >& in https://github.com/git/git/blob/d62dad7a7dca3f6a65162bf0e52cdf6927958e78/t/test-lib.sh and their interaction with similar uses in https://github.com/git/git/blob/d62dad7a7dca3f6a65162bf0e52cdf6927958e78/t/test-lib-functions.sh.

Sorry to necro the thread — I was hoping to do some fd magic in fish and wound up here via #3948.

[faho]

Okay, this no longer happens.

[zanchey]

Those file descriptors are still open, though, so sending data to them could corrupt internal state:

> ls -lh /proc/$fish_pid/fd
Permissions Size User    Group Date Modified Name
lrwx------    64 zanchey gumby 12 Jan 21:09  0 -> /dev/pts/51
lrwx------    64 zanchey gumby 12 Jan 21:09  1 -> /dev/pts/51
lrwx------    64 zanchey gumby 12 Jan 21:09  2 -> /dev/pts/51
lr-x------    64 zanchey gumby 12 Jan 21:09  3 -> /home/wheel/zanchey
lr-x------    64 zanchey gumby 12 Jan 21:09  4 -> pipe:[1323274685]
l-wx------    64 zanchey gumby 12 Jan 21:09  5 -> pipe:[1323274685]
lr-x------    64 zanchey gumby 12 Jan 21:09  6 -> pipe:[1323268796]
l-wx------    64 zanchey gumby 12 Jan 21:09  7 -> pipe:[1323268796]
lrwx------    64 zanchey gumby 12 Jan 21:09  8 -> /run/user/11251/fish_universal_variables.notifier

[ridiculousfish]

It looks like this does still happen - you can write to arbitrary fds within fish. We should prevent this.

[zanchey]

Should be fixed by 11a373f? @ridiculousfish

[zanchey]

Looks like.

[ridiculousfish]

Yeah should be fixed!

[zanchey]

This commit is reverted in the upcoming 3.2.1, so I am reopening the issue.