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

Weird interaction with ncurses #42

Closed
sigmaSd opened this issue Jul 21, 2020 · 8 comments
Closed

Weird interaction with ncurses #42

sigmaSd opened this issue Jul 21, 2020 · 8 comments

Comments

@sigmaSd
Copy link

sigmaSd commented Jul 21, 2020

Hi, after hooking to SIGWNICH with signal-hook then dropping the Signal for clean up, ncurses is unable to receive resize events, here is the code:

use ncurses::{getch, initscr};
use signal_hook::iterator::Signals;

fn main() {
    let mut signals = Signals::new(&[signal_hook::SIGWINCH]).unwrap();
    signals.close();
    drop(signals);

    initscr();
    loop {
        if getch() == 410 { // 410 is resize event
            panic!("this wont happen")
        }
    }
}

panic wont happen when resizing the window.

This arose after debugging crossterm-rs/crossterm#447

@vorner
Copy link
Owner

vorner commented Jul 21, 2020

🤔 It is interesting this has any effect even though initscr is called after all handling of signal hook. But it might be related, as SIGWINCH is the thing that gets called when there's a resize event (which maybe ncurses translates?).

Can you check if it also happens if you simply register a sigaction(SIGWINCH, empty_hadler /*not SIG_DFL*/, prev)? instead of juggling with signal hook?

@sigmaSd
Copy link
Author

sigmaSd commented Jul 21, 2020

I tried it like this

use ncurses::{getch, initscr};
use libc::sigaction;

fn main() {

	unsafe {
	sigaction(28, std::mem::zeroed(),std::mem::zeroed());
	}

    initscr();
    loop {
        if getch() == 410 { // 410 is resize event
            panic!("this wont happen")
        }
    }
}

The resize worked (it panicked)

strace: rt_sigaction(SIGWINCH, NULL, NULL, 8) = 0

Is this what you meant by an empty handler?

@sigmaSd
Copy link
Author

sigmaSd commented Jul 21, 2020

With this:

use libc::c_int;
use libc::sigaction;
use ncurses::{getch, initscr};

fn main() {
    unsafe {
        let mut a: libc::sigaction = std::mem::zeroed();
        a.sa_sigaction = handler as usize;

        sigaction(28, &a, std::mem::zeroed());

        //dbg!(&a as *const u8);
        //	panic!();

        initscr();
        loop {
            if getch() == 410 {
                // 410 is resize event
                panic!("this wont happen")
            }
        }
    }
}

extern "C" fn handler(sig: c_int) {
    return;
}

Resize event is not detected.

@sigmaSd
Copy link
Author

sigmaSd commented Jul 21, 2020

From here https://stackoverflow.com/questions/9302464/how-do-i-remove-a-signal-handler
I tried this

use libc::c_int;
use libc::sigaction;
use ncurses::{getch, initscr};

fn main() {
    unsafe {
        let mut a: libc::sigaction = std::mem::zeroed();
        a.sa_sigaction = handler as usize;

        sigaction(28, &a, std::mem::zeroed());

		let mut remove: libc::sigaction = std::mem::zeroed();
		remove.sa_sigaction = libc::SIG_DFL;
		
		sigaction(28, &remove, std::mem::zeroed());

        initscr();
        loop {
            if getch() == 410 {
                // 410 is resize event
                panic!("this wont happen")
            }
        }
    }
}

extern "C" fn handler(sig: c_int) {
    return;
}

which works.
Is this a correct solution?

@vorner
Copy link
Owner

vorner commented Jul 21, 2020

The latter one was what I meant.

Then I think this is not signal-hooks fault, that it works as expected (or at least consistently with whatever signal handling).

Even on unregistering everything, signal-hook doesn't unregister its signal handler. It's because signal-handling libraries tend to „chain“ ‒ when registering their signal handler, they store the previous one and call that one in a chain. And it is not reasonably possible to unregister from that chain and any attempt to unregistration could break other libraries, so signal-hook just doesn't attempt and keeps its empty handler. I think it is even documented.

My guess is ncurses somehow doesn't work if anyone touches the SIGWINCH signal first (no idea why).

I'm going to close this based on the above experiment. If you believe this is wrong, feel free to reopen, but please specify what you'd want to happen and how.

https://docs.rs/signal-hook/0.1.16/signal_hook/fn.unregister.html#warning

@vorner vorner closed this as completed Jul 21, 2020
@sigmaSd
Copy link
Author

sigmaSd commented Jul 21, 2020

Thanks for detailed explanation!
I understand and I think a pragmatic solution for libraries that deepens on signal-hook (like crossterm) might be to reexport signal_hook_cleanup::cleanup_signal.

@vorner
Copy link
Owner

vorner commented Jul 21, 2020

Might be, but be careful with that one. It's irreversible (you can't register the same signal with signal hook again any more; this might be fixable, though) and global (it would kill hooks of any other libraries you're not aware of, so it probably should be left for the end application to call).

@sigmaSd
Copy link
Author

sigmaSd commented Jul 21, 2020

Ok got it, maybe just documenting the interaction is enough for now, thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants