Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

/dev/tty does not work on macOS with kqueue #500

Closed
acidghost opened this issue Oct 23, 2020 · 7 comments · Fixed by #735
Closed

/dev/tty does not work on macOS with kqueue #500

acidghost opened this issue Oct 23, 2020 · 7 comments · Fixed by #735

Comments

@acidghost
Copy link
Contributor

Describe the bug
As described in tokio-rs/mio#1377, kqueue does not play well with /dev/tty. This is a problem for crossterm (as in #407), being unable to work properly.

To Reproduce
See reproducer in tokio-rs/mio#1377.

Expected behavior
Do not fail to poll on /dev/tty.

OS
MacOs.

Terminal/Console
Any.

Possible solutions
The general solution is to use select instead of kqueue or epoll (poll should not work either). Changing Mio to support a select-based Poll requires major refactoring that is probably never going to happen. Given that mio::Poll is used in crossterm only (AFAIK) to poll SIGWINCH and /dev/tty, I suggest dropping the dependency on Mio and implement a select-based polling system with libc or nix.

Two major problems:

  • how to poll from fd and signal on non-linux (w/o signalfd);
  • how does it play with the waker system?

I'd be open to make a PR. Any suggestion?

@acidghost
Copy link
Contributor Author

To handle signals we could use the signal_hook crate with pipes and select on /dev/tty and the read end of the pipe.

I've never used Rust asynchronous programming features (i.e. async/await) so I'm a bit lost for the event-stream feature.

@TimonPost
Copy link
Member

TimonPost commented Oct 23, 2020

That is sad to hear :(

There are 3 things at stake when speaking about rewrites:

  • Crossterm uses the signal crate with the mio extension for terminal size.
  • Crossterm uses MIO for polling event readiness on unix systems.
  • Crossterm has a Waker type which throws an event and wakes up a pending poll operation from MIO.

Also as for async input system, only the Waker and EventSource needs to function correctly. All other async logic is not of importance in this issue (as far I know).

There are some options here. First, given that this use case of /dev/tty only doesnt work on macos we could limit this feature and make macos use STDOUT by default. Which might bring some sad faces along.

Secondly, rewrite/extend the input system. The area of code I'm talking about can be found here

It 'could' be easy to replace the system. Since the logic responsible for all input/eventing logic is hidden behind a trait that is currently implemented for windows and unix. We can create a new implementation called MacOsEventSource for example. If this implementation works well, according to that which is expected behavior, it could be added here with a cfg(macos) macro.

Steps to create a new input system

  • Create a new EventSource like MacOsEventSource
  • Implement the try_read functionality, this function must be able to:
    • Wait for input for a certain duration
    • Return Some (TerminalResizeEvent, Input) or None if the duration elapsed.
    • Being able to wake up the read
  • Add the newly created EventSource to the InternalEventReader` here. This should make crossterm use a select based approach and therefore fixing all problems for macos.

This select based EventSource could serve as the default implementation for all Unix systems or we can limit it to MacOs (as spoken about above).

As far select goes, I am not that familiar with it and cant estimate how difficult it is to make it work within the EventSource trait.

@acidghost
Copy link
Contributor Author

There are 3 things at stake when speaking about rewrites:

* Crossterm uses the signal crate with the mio extension for [terminal size](https://github.com/crossterm-rs/crossterm/blob/master/src/event/source/unix.rs#L129).

* Crossterm uses MIO for polling event readiness on unix systems.

* Crossterm has a `Waker` type which throws an event and wakes up a pending `poll` operation from MIO.

All of the above can be done w/o Mio, with just the signal_hook crate, select syscall and pipes.

There are some options here. First, given that this use case of /dev/tty only doesnt work on macos we could limit this feature and make macos use STDOUT by default. Which might bring some sad faces along.

I'm guessing you meant STDIN. The problem there, which is one I already faced while using crossterm, is that a downstream crate may want to read, e.g., a file from standard input.

This select based EventSource could serve as the default implementation for all Unix systems or we can limit it to MacOs (as spoken about above).

Indeed, an event source using select would work just fine on all UNIX systems: it's not used for polling from 100s of FDs and is the most supported syscall for polling (since the '80s). I'd be more keen to drop the dependency on Mio and rewrite the UnixEventSource.

Which of the two options would you prefer?

Also, I don't see tests around. How is this crate tested?

@TimonPost
Copy link
Member

TimonPost commented Oct 23, 2020

Testing is mainly done with hand :). Just press some keys and see if it works. It is very hard to write tests for the input module since terminals differ from key combinations they support and we need to physically interact with the STDOUT/STDIN/ or /dev/tty. Which is not available in a CI environment. So its tricky.

If we can drop the MIO and are able to implement the same thing with 'select' I'm all the way in. It would make crossterm way smaller. So that's cool. Anyhow, as far I can tell it is quite easy to create a custom EventSource and create an implementation. If that's done we can test it and if it works we can drop it and switch to it completely. The async part should not be a deal as long the EventSource does its job well as described.

@acidghost
Copy link
Contributor Author

Great, I'll work on a PR.

Do you have plans to merge #407 or shall I open a new PR?

That would just use tty_fd instead of STDIN in crossterm::terminal. E.g. here:

wrap_with_result(tcgetattr(STDIN_FILENO, &mut termios))?;

@TimonPost
Copy link
Member

TimonPost commented Oct 24, 2020

Nice!

This branch is waiting for the mac problem to be fixed, though that's what you going to do now. Thus I think closing the branch is better, it contains lots of debug code, and the important changes are not that big of a change. We should use the tty_fd function. And this function should first try /dev/tty but if not possible it should try stdin at the following places:

acidghost added a commit to acidghost/crossterm that referenced this issue Oct 24, 2020
acidghost added a commit to acidghost/crossterm that referenced this issue Oct 24, 2020
amtoine pushed a commit to amtoine/nu_plugin_explore that referenced this issue Apr 13, 2024
Copied ver batim from sigoden/aichat#264.

### Background

- Identical bug: sigoden/aichat#257
- Request for docs: crossterm-rs/crossterm#849
- Similar bug:
crossterm-rs/crossterm#644 (comment)
- Source bug: crossterm-rs/crossterm#500
- Fix (enabled in this PR): crossterm-rs/crossterm#735

### Error

```
│hread '<unnamed>' panicked at src/event.rs:46:45:                                                             │
│o events available: Custom { kind: Other, error: "Failed to initialize input reader" }                        │
```

### Backtrace

In case of regression:

```diff
┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│key                             bfield                                                                                                          shape           │
│version                          0.91.0                                                                                                         string          │
│branch                          c                                                                                                               string          │
│commit_hash                     d                                                                                                               string          │
│build_os                         macos-x86_64                                                                                                   string          │
│build_target                     x86_64-apple-darwin                                                                                            string          │
│rust_version                     rustc 1.76.0 (07dca489a 2024-02-04) (Homebrew)                                                                 string          │
│cargo_version                    cargo 1.76.0                                                                                                   string          │
│build_time                       2024-03-05 20:28:40 +00:00                                                                                     string          │
│build_rust_channel               release                                                                                                        string          │
│allocator                        mimalloc                                                                                                       string          │
│features                         dataframe, default, extra, sqlite, trash, which, zip                                                           string          │
│installed_plugins                nu_plugin_explore                                                                                              string          │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
│                                                                                                                                                                │
└────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
cell path: $.version
 NORMAL                                                                                 i to INSERT | hjkl to move around | p to peek | t to transpose | q to quit   0:        0x101a6ab65 - std::backtrace_rs::backtrace::libunwind::trace::he87ba3c236c7ad5f
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:        0x101a6ab65 - std::backtrace_rs::backtrace::trace_unsynchronized::h3ad5d899409e49ee
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x101a6ab65 - std::sys_common::backtrace::_print_fmt::hb1551f966d2dd86d
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:68:5
   3:        0x101a6ab65 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hbd71adb7a72f4105
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:44:22
   4:        0x101a8c993 - core::fmt::rt::Argument::fmt::h4224d647cce844bf
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/rt.rs:142:9
   5:        0x101a8c993 - core::fmt::write::h30346430340bc336
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/mod.rs:1120:17
   6:        0x101a67a8e - std::io::Write::write_fmt::heb3d6316c565d5b1
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/io/mod.rs:1810:15
   7:        0x101a6a939 - std::sys_common::backtrace::_print::hc99e5bf521524ac2
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:47:5
   8:        0x101a6a939 - std::sys_common::backtrace::print::h67e51ff2e3d5cfbd
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:34:9
   9:        0x101a6c575 - std::panicking::default_hook::{{closure}}::hc666c9a55318d1f1
  10:        0x101a6c2ee - std::panicking::default_hook::hf980b1da49948523
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:292:9
  11:        0x100e53d54 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h09d0f3f719801ec9
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2029:9
+ 12:        0x100e4c81d - nu_plugin_explore::tui::Tui<B>::init::{{closure}}::h92cf9edd04f48f7a
+                              at /Users/texas/Developer/git/nu_plugin_explore/src/tui.rs:45:13
  13:        0x101a6cb75 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h3a63db2ca77cedb5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2029:9
  14:        0x101a6cb75 - std::panicking::rust_panic_with_hook::h683bce980186bbbe
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:783:13
  15:        0x101a6c904 - std::panicking::begin_panic_handler::{{closure}}::ha6dbd11ba0ec8af1
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:657:13
  16:        0x101a6b059 - std::sys_common::backtrace::__rust_end_short_backtrace::h889430bddc786c98
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:171:18
  17:        0x101a6c642 - rust_begin_unwind
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
  18:        0x101ab3f75 - core::panicking::panic_fmt::hff768cef35397791
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
  19:        0x101ab4535 - core::result::unwrap_failed::h951d84d71b0bada2
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1649:5
  20:        0x100e86f0f - core::result::Result<T,E>::expect::h46e49f62a7f5b691
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1030:23
+ 21:        0x100e39983 - nu_plugin_explore::event::EventHandler::new::{{closure}}::h6a786ba814d713f3
+                              at /Users/texas/Developer/git/nu_plugin_explore/src/event.rs:46:24
  22:        0x100e6d68d - std::sys_common::backtrace::__rust_begin_short_backtrace::h0d557fbf2c6545a9
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:155:18
  23:        0x100e4f670 - std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}::h49806568e55dc3ef
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/thread/mod.rs:529:17
  24:        0x100e39260 - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::ha7914519392245f9
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panic/unwind_safe.rs:272:9
  25:        0x100e3dee0 - std::panicking::try::do_call::h77e6e80f923ba765
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  26:        0x100e3df7d - ___rust_try
  27:        0x100e3de59 - std::panicking::try::hb72a1d854f6956b8
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  28:        0x100e4f4ad - std::panic::catch_unwind::h35d42a1f268a9228
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  29:        0x100e4f4ad - std::thread::Builder::spawn_unchecked_::{{closure}}::hd42e13ba971d3218
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/thread/mod.rs:528:30
  30:        0x100e74451 - core::ops::function::FnOnce::call_once{{vtable.shim}}::h6b49c774539034b2
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:250:5
  31:        0x101a712e9 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hef77fdfabbdc0490
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2015:9
  32:        0x101a712e9 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hd4d34ecf6438f9ac
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2015:9
  33:        0x101a712e9 - std::sys::unix::thread::Thread::new::thread_start::hcdd70219a480b010
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys/unix/thread.rs:108:17
  34:     0x7ff81213918b - __pthread_start
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants