ssh: Do not leak ssh sessions on errors#1540
Conversation
|
Some more background infos: We are using the |
|
This looks reasonable at a quick glance. Any chance you could create a functional test case for this?... |
|
Excellent. This PR actually fixes #1425 issue in proper way. That is why fleet tried to send several agent requests... @antrik functional tests are already written (#1492). What we need to do it to revert my P.S. I had feeling that I fixed that wrong, but didn't have enough time to investigate. |
|
Just rereading my comment I think I missed an info: this PR fixed the We fixed that then by adding a mutex to the |
|
@zeisss false alarm. my tests succeed because my VM had not enough RAM (256). And functional tests returned positive answer for some reason. My fix should not be reverted. Your and my fixes supplement each other. |
|
lgtm. Thank you! |
|
Wh00p wh00p. |
|
👍 nice |
Hello there. We recently ran into a problem where we were not able to do proper connection handling across ssh tunnels. It turned out that this was partially caused by some buggy code on our side in combination with the things being fixed here in this PR. In case ssh sessions are created, we need to make sure to close them properly even in error cases. The fix proposed here works for us. What do you guys think? Thanks to @zeisss helping out debugging and fixing this issue.