After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 331056 - vte should wait for gnome-pty-helper to exit cleanly
vte should wait for gnome-pty-helper to exit cleanly
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.11.x
Other All
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-02-13 21:33 UTC by Robert Basch
Modified: 2006-02-13 23:20 UTC
See Also:
GNOME target: ---
GNOME version: 2.7/2.8


Attachments
Wait for gnome-pty-helper to exit cleanly (1.28 KB, patch)
2006-02-13 21:36 UTC, Robert Basch
none Details | Review

Description Robert Basch 2006-02-13 21:33:02 UTC
Please describe the problem:
We have noticed that gnome-terminal usually leaves a stale utmp record behind
at the end of sessions.  The problem seems to be due to the vte library's
termination logic, in _vte_pty_stop_helper() -- vte closes its pipe to the
helper, then kills the helper with SIGTERM.  When the helper detects that the
pipe has closed, it attempts to exit cleanly, including updating utmp, but
usually gets killed by the SIGTERM before it can complete this.

Steps to reproduce:
1. Run gnome-terminal, and note its utmp entry (e.g. using "who").
2. Logout of the session.
3. From a (non-GNOME) terminal session, (e.g. virtual console or ssh login),
   look at the utmp file; most of the time, the now-stale gnome-terminal
   entry remains.


Actual results:
The utmp record is not cleaned up at session end.

Expected results:
The utmp entry should be cleaned up -- e.g. marked as a "DEAD PROCESS", etc.

Does this happen every time?
No, it's a race condition (see description), but does happen most of the time
for us.

Other information:
I will follow up with a patch which works around the problem.
Comment 1 Robert Basch 2006-02-13 21:36:43 UTC
Created attachment 59295 [details] [review]
Wait for gnome-pty-helper to exit cleanly

This patch makes vte wait for up to 5 seconds for gnome-pty-helper to exit
cleanly, before killing it, to give it a chance to update utmp properly.
Comment 2 Behdad Esfahbod 2006-02-13 22:10:38 UTC
I cannot agree.  In gnome-pty-helper.c there is:

        signal (SIGHUP, exit_handler);
        signal (SIGTERM, exit_handler);

where:

exit_handler (int signum)
{
        shutdown_helper ();
        _exit (1);
}

and:

static void
shutdown_helper (void)
{
        pty_info *pi;

        for (pi = pty_list; pi; pi = pty_list)
                shutdown_pty (pi);
}

and:

static void
shutdown_pty (pty_info *pi)
{
        if (pi->utmp || pi->wtmp || pi->lastlog)
                if (pi->data)
                        write_logout_record (pi->data, pi->utmp, pi->wtmp);

        pty_remove (pi);
}
Comment 3 Behdad Esfahbod 2006-02-13 22:13:26 UTC
And indeed, shutdown helper is what is called when the pipe is closed:

                res = n_read (STDIN_FILENO, &op, sizeof (op));

                if (res != sizeof (op)) {
                        shutdown_helper ();
                        return 1;
                }

So, the only race condition that I see here is if it's trying to shutdown AND a SIGTERM comes in and runs shutdown_helper again.  That's definitely a race condition, but shouldn't have the effects you are observing.  I'm going to reorder code to avoid that.
Comment 4 Behdad Esfahbod 2006-02-13 22:24:41 UTC
Hopefully this will do some good.  Can you test please?

2006-02-13  Behdad Esfahbod  <behdad@gnome.org>

        * gnome-pty-helper/gnome-pty-helper.c: Fix race condition when
        shutdown_helper may be called again from the signal handler when
        it's already running. (bug #331056)

Comment 5 Robert Basch 2006-02-13 22:31:10 UTC
Ah, the version we're running (0.11.11) does not have the exit_handler logic
in gnome-pty-handler, so we see the resulting stale utmp entries.  Sorry for
not noticing that before.  I'll test your new patch and report back soon...
Comment 6 Behdad Esfahbod 2006-02-13 22:33:13 UTC
Ok, closing then.  Reopen if CVS still has problems.  Thanks.
Comment 7 Robert Basch 2006-02-13 23:20:32 UTC
OK, I tested with the exit_handler() patch, plus your latest patch,
and all looks well.  Thanks!