RFC: Support for libssh2_session_callback_set2

[kdkasad]

The goal

I am trying to get SSH agent forwarding working in Rust with the ssh2 library. Currently, the crate is missing some of the pieces needed to do this (#200). libssh2 supports everything necessary (see libssh2/libssh2#1503).

In order to use SSH agent forwarding, a callback needs to be established using libssh2_session_callback_set2. This callback is called when packets of a certain type are received. This callback function is supposed to be passed a reference to the session object.

The problem

The above works fine in C. However, it would cause a deadlock if this were to be implemented in the ssh2 crate, because:

1. A function such as (Session as Read)::read() is called. This locks the session's mutex.
2. (Session as Read)::read() calls libssh2_channel_read_ex. This function receives a certain type of packet and calls our callback.
3. The callback function is passed the reference to the session object.
4. A method on the session is called, attempting to lock the already-locked mutex. Deadlock ensues.

Possible solutions

I see three possible solutions to this currently:

1. Make the mutex the user's problem.

   I.e., take the mutex out of the session object and make the user of the library keep their own mutex. The user would lock the mutex, then call whatever functions they need. The callback would be passed the session object directly, sans mutex. Since the callback is called in the same thread, there are no issues with concurrent access here.

   This approach works, but requires way too much effort, as a major version bump would definitely be needed, and all code wanting to use the new version of ssh2 would need to be re-written at least slightly.

2. Switch from a regular mutex to a re-entrant one which allows multiple simultaneous locks as long as they're from the same thread.

   This would likely be the best approach, as we should just be able to swap out the type of mutex used without having to make any other changes. We're already using the parking_lot crate, so we can just use their ReentrantMutex type.

   Edit: This actually won't work, because the ReentrantMutex does not allow mutable references to the data it owns.

3. Just don't pass the session object to the callback function.

   This is by far the easiest approach, but it is also the worst one. It limits the functionality of the crate, as it makes impossible some things that are possible with the underlying libssh2 library.

   Edit: It turns out this won't work either, because some of the callbacks do require a Channel in order to be useful at all. The Channel contains an Arc<Mutex<Session>>, so channel operations would also deadlock.

4. Wrap SessionInner in a temporary "fake" Session before giving it to the callback.

   We can keep a pointer to the SessionInner object in the session's abstract. When the callback is invoked, it first runs a "bridge" function that calls the actual callback passed by the user. This bridge function can take the SessionInner and wrap it in a new mutex to create a Session object.

   This could cause problems if the callback tries to clone store this Session and store it past the execution lifetime of the callback closure.

Wrap-up

I am going to try to implement callbacks for SSH agent forwarding support for my own use, likely using approach 3 above (because the agent callback doesn't make use of the session). I would be happy to upstream my work, so I would be grateful for guidance on which of the possible approaches would be favored by the maintainers (and community). If there are other solutions or comments, I'm all ears.