gopasspw / gopass

The slightly more awesome standard unix password manager for teams
https://www.gopass.pw/
MIT License
5.93k stars 496 forks source link

`gopass show -c` process never stops #1666

Closed freijon closed 2 years ago

freijon commented 3 years ago

Summary

I tried running the command described in setup.md: gopass ls --flat | dmenu | xargs --no-run-if-empty gopass show -c. This apparently launches two gopass commands which run forever.

Steps To Reproduce

Expected behavior

45 seconds after the command has been executed all gopass processes should stop.

Environment

AnomalRoil commented 3 years ago

What happens if you run gopass show -c nameofanentry ? Does it also keep running ?

freijon commented 3 years ago

The command starts and keeps running forever. However, there is no message about 45 seconds.

EDIT: I'm using Wayland, maybe that's the issue?

ghost commented 2 years ago

@AnomalRoil, @dominikschulz: I'm able to reproduce this issue, but using different steps:

  1. I assume you already have password store with secret called "test".
  2. Run gopass
  3. In gopass built-in shell, run show -c test few times in a row without waiting 45 seconds.
  4. Switch to another terminal tab and run pgrep --list-full gopass. Example output will look like this:
    1000 gopass
    1001 gopass
    1002 gopass
    1003 gopass unclip --timeout 45

Process with PID 1000 is gopass built-in shell, PIDs 1001 and 1002 are zombie processes left after gopass unclip and PID 1003 is most recent gopass unclip. After 45 seconds, PID 1003 becomes zombie as well. Those processes are there until parent process exits.

I'm not Go developer, but I think I found the solution to this issue. Here is Stack Overflow question about similar issue. I also found that Process.Kill "does not wait until the Process has actually exited." and you need to Process.Wait for those processes to exit after they get killed. Basically they are zombies until the parent process reuqests exit status from them.

I think that modifying killProc function to make it wait after killing the process should be enough to fix this issue.

dominikschulz commented 2 years ago

@fm2mwvtsp8vqtuy5 Thanks for the detailed explanation. Unfortunately I was not able to reproduce. At least on my system (Linux) killPrecedessors does seem to take care of any lingering processes.

But you are right that we're not using Process.Kill according to documentation, so we'll fix that :+1:

ghost commented 2 years ago

@dominikschulz: I'm glad I could help you and I'm happy to see that you fixed it so quick.