Bug 5051 - chg: press Ctrl-Z in `hg commit -i` will mass up the terminal
Summary: chg: press Ctrl-Z in `hg commit -i` will mass up the terminal
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: chg (show other bugs)
Version: default branch
Hardware: PC Linux
: normal bug
Assignee: Jun Wu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-19 13:07 UTC by Jun Wu
Modified: 2016-03-01 00:00 UTC (History)
3 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jun Wu 2016-01-19 13:07 UTC
(not found "chg" component so "Mercurial" is chosen)

Repro:

1. Enable crecord
2. Make working copy dirty
3. `chg commit -i`
4. Press `Ctrl-Z`

The issue is caused by 2 reasons:

1. chg client does not handle SIGTSTP yet. This is easy to fix, just resending the signals to the (forked) server.
2. chg server, if runs ncurses, will use ncurses SIGTSTP handler. In that handler, resending SIGTSTP is expected to stop the process (https://github.com/gittup/ncurses/blob/gittup/ncurses/tty/lib_tstp.c#L209) but it will fail to do so because it cannot pass kernel "is_current_pgrp_orphaned" check (http://lxr.free-electrons.com/source/kernel/exit.c#L215)
Comment 1 HG Bot 2016-02-07 02:01 UTC
Fixed by https://selenic.com/repo/hg/rev/83fc0c055664
Jun Wu <quark@fb.com>
chgserver: create new process group after fork (issue5051)

This is to make SIGTSTP work. Before the patch, the server process group is
considered "orphaned" and will ignore SIGTSTP, SIGTTIN, SIGTTOU, according to
POSIX. See the comment above `will_become_orphaned_pgrp` in `kernel/exit.c`
from Linux 4.3 for details.

SIGTSTP is important if chgserver runs some ncurses commend like `commit -i`.
Ncurses has its own SIGTSTP handler which will do the following:

  1. Clean the screen
  2. Stop itself by resending SIGTSTP to itself
  3. Restore the screen

If SIGTSTP is ignored, step 2 will be a noop, which means the process cannot
be suspended properly.

In order to make things work, chg client needs to forward SIGTSTP and SIGCONT
to server as well.

(please test the fix)
Comment 2 Bugzilla 2016-02-15 00:00 UTC
Bug was set to TESTING for 7 days, resolving
Comment 3 HG Bot 2016-02-22 15:32 UTC
Fixed by https://selenic.com/repo/hg/rev/65d24ca35496
Yuya Nishihara <yuya@tcha.org>
chg: forward job control signals to worker process (issue5051)

This is necessary to suspend/resume long pulls, interactive curses session,
etc.

The implementation is based on emacsclient, but our version doesn't test if
chg process is foreground or not before propagating SIGCONT. This is because
chg isn't always an interactive session. If we copy the SIGTTIN/SIGTTOU
emulation from emacsclient, non-interactive session can't be moved to a
background job.

  $ chg pull
  ^Z
  suspended
  $ bg %1
  [1] continued
  [1] suspended (tty input)  # wrong

https://github.com/emacs-mirror/emacs/blob/0e96320/lib-src/emacsclient.c#L1094

(please test the fix)
Comment 4 Bugzilla 2016-03-01 00:00 UTC
Bug was set to TESTING for 7 days, resolving