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

Closing a tab or the window does not result in a logout #7826

Closed
doomchild opened this issue Oct 5, 2020 · 3 comments
Closed

Closing a tab or the window does not result in a logout #7826

doomchild opened this issue Oct 5, 2020 · 3 comments
Labels
Issue-Question For questions or discussion Needs-Tag-Fix Doesn't match tag requirements Resolution-Answered Related to questions that have been answered Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons.

Comments

@doomchild
Copy link

I have Debian installed (uname -a returns Linux Arcadia 4.19.104-microsoft-standard #1 SMP Wed Feb 19 06:37:35 UTC 2020 x86_64 GNU/Linux), and I was in the process of getting all of my SSH stuff setup when I noticed that my .bash_logout (where I kill the instance of ssh-agent that was started on login) wasn't getting executed if I closed a tab or exited Windows Terminal. After a couple of opens and closes, I've got five or six instances of ssh-agent -s running.

If I explicitly call logout, the window closes and I can open a new one to see that the previous instance was correctly killed as desired.

Environment

Windows build number: 10.0.19042.0
Windows Terminal version (if applicable): 1.1.2233.0

Steps to reproduce

Add the following to ~/.bashrc:

if [ -z "$SSH_AUTH_SOCK" ] ; then
  eval `ssh-agent -s`
fi

Add the following to ~/.bash_logout

if [ -n "$SSH_AUTH_SOCK" ] ; then
  eval `/usr/bin/ssh-agent -k`
fi

Close Windows Terminal.
Open Windows Terminal.
Close Windows Terminal.
Open Windows Terminal.

Execute ps aux | grep ssh-agent.

Expected behavior

There should be only one instance of ssh-agent running.

Actual behavior

There are multiple instances of ssh-agent still running.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 5, 2020
@DHowett
Copy link
Member

DHowett commented Oct 5, 2020

Is this not what happens when you close an xterm or gnome-terminal? I am not certain that shells typically react to SIGHUP by processing their logout scripts.

Does this work as expected elsewhere?

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 5, 2020
@doomchild
Copy link
Author

My Debian box autostarts ssh-agent via x-session-manager, so I can't test this exact scenario. However, one big difference is that if I try to logout on that box, I get bash: logout: not login shell: use 'exit', whereas on WSL2, if I enter logout, the tab closes (or the window if it's the only tab). If Windows Terminal is opening a login shell, shouldn't .bash_logout be run when the window is closed?

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Oct 5, 2020
@DHowett
Copy link
Member

DHowett commented Oct 5, 2020

So, alright. There's a combination of issues here.

  • wsl -d X starts your login shell as a login shell (automatically)
  • It is not standard behavior for a shell to run logout scripts when its controlling terminal exits

This behavior can be recapitulated in gnome-terminal if you toggle the "start ... as login shell" option: exiting the tab unceremoniously does not result in .bash_logout running.

I'm going to close this bug as by-design, but with a quick suggestion:

It may be possible to cache the output of ssh-agent such that you don't need to rely on a shell to exit gracefully and tear down the running instance. My personal agent startup looks a bit like...

function {
        function _agent_add_keys() {
                ssh-add &> /dev/null
                ssh-add ~/.ssh/id_rsa_howett.net &> /dev/null
        }

        if [[ ! -z "${SSH_AUTH_SOCK}" ]]; then
                if ! ssh-add -l &> /dev/null; then
                        _agent_add_keys
                fi
                unfunction _agent_add_keys
                return
        fi

        local _agent_env
        _agent_env="$HOME/.ssh/agent-env-$HOST"
        function _start_agent() {
                ssh-agent -s > "${_agent_env}"
                chmod 600 "${_agent_env}"
                . "${_agent_env}" &> /dev/null
                _agent_add_keys
        }


        ssh-add -l &> /dev/null
        if [[ $? -eq 2 ]]; then
                [[ -f "${_agent_env}" ]] && . "${_agent_env}" &> /dev/null
                ssh-add -l &> /dev/null
                if [[ $? -eq 2 ]]; then
                        _start_agent
                fi
        fi

        unfunction _start_agent
        unfunction _agent_add_keys
}

(admittedly, this is for zsh instead of bash.) It caches the agent info in ~/.ssh/agent-env-LIBRA or whatever my machine name suggests it should be, and each instance of my shell imports that info. It detects that the agent has exited because it'll fail to add a key. If it does so, it restarts it.

This seems a little cleaner than having one agent per shell instance, and presuming that I launch a lot of shells it's a reasonable savings of resources spent.

@DHowett DHowett closed this as completed Oct 5, 2020
@DHowett DHowett added Issue-Question For questions or discussion Resolution-Answered Related to questions that have been answered and removed Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 5, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Oct 5, 2020
@DHowett DHowett added the Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons. label Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Question For questions or discussion Needs-Tag-Fix Doesn't match tag requirements Resolution-Answered Related to questions that have been answered Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons.
Projects
None yet
Development

No branches or pull requests

2 participants