session: Add Session::free to avoid memory leak on drop

[pbar1]

Mitigates #220 by adding a new function Session::free which is a wrapper for libssh2_session_free.

Since libssh2_session_free also sometimes performs IO (and thus return EAGAIN when in nonblocking mode), the thinking here is to provide an escape hatch for nonblocking mode to explicitly shutdown/free the session.

Prior to this patch, when used within the context of a (closed source, unfortunately) bulk SSH connection tool connecting to 30k hosts in nonblocking mode, ssh2 would leak sessions to the tune of about 1.5 MB total. Using this patch and calling Session::free explicitly reduces that to 0 MB when run under Valgrind.

Notes:

• Does not change the behavior of Session::drop aside from guarding it with the freed bool. There was an example in the linked issue of taking the TCP stream and dropping it manually, but this does not do that. Should make for a more backwards compatible change.
• Does not guard each method with a check for SessionInner.freed either. There was some mention of that in the linked issue as well, requiring changing Arc<Mutex<SessionInner>> -> Arc<Mutex<Option<SessionInner>>>. If that's desired, I can make that change on top of this one too. It would be more correct/sound, but a more invasive change.

[yodaldevoid]

Thanks for the PR. I'll take a closer look at this tomorrow.

Just a nit, but please re-add the trailing newline that was removed in your patch.

A quick look at CI says I need to do a pass on tests, but I don't think that has anything to do with your changes.

[yodaldevoid]

I think it would make more sense for this method to be pub fn free(self) -> Result<(), (Self, Error)>. This would ensure the session is not re-used after it is freed when it is freed successfully.

Session::drop wouldn't need to be modified if the Session was std::mem::forgeted when successfully freed.

[pbar1]

Nice, that's a great API. I played around with it, then realized that it could still result in a use-after-free with the fact that the Session could have been cloned, meaning multiple Arc-based references to the SessionInner.

I realized Sftp::shutdown uses Arc::try_unwrap to perform an analogous thing when it has the last remaining reference - to be consistent, I implemented a similar thing here for Session::free, and I think it's safe from those use-after-frees now.

On std::mem::forget and avoiding changes to Drop - IMO changing Drop is preferable now that we're using Arc::try_unwrap, because it lets SessionInner still drop normally and clean up the TCP stream while only suppressing the second libssh2_session_free.

Thnks for the review!