Channel, Sftp no longer borrow Session

Instead the internal session is kept alive via Rc

Refs: https://github.com/alexcrichton/ssh2-rs/issues/53

6 files changed, 108 insertions, 89 deletions

diff --git a/src/channel.rs b/src/channel.rs
index 85c6576..febf3d6 100644
--- a/src/channel.rs
+++ b/src/channel.rs
@@ -2,10 +2,10 @@ use libc::{c_char, c_int, c_uchar, c_uint, c_ulong, c_void, size_t};
 use std::cmp;
 use std::io::prelude::*;
 use std::io::{self, ErrorKind};
+use std::rc::Rc;
 use std::slice;
 
-use util::SessionBinding;
-use {raw, Error, Session};
+use {raw, Error, SessionInner};
 
 /// A channel represents a portion of an SSH connection on which data can be
 /// read and written.
@@ -15,16 +15,33 @@ use {raw, Error, Session};
 /// implements the `Reader` and `Writer` traits to send and receive data.
 /// Whether or not I/O operations are blocking is mandated by the `blocking`
 /// flag on a channel's corresponding `Session`.
-pub struct Channel<'sess> {
+pub struct Channel {
     raw: *mut raw::LIBSSH2_CHANNEL,
-    sess: &'sess Session,
+    sess: Rc<SessionInner>,
     read_limit: Option<u64>,
 }
 
+impl Channel {
+    pub(crate) fn from_raw_opt(
+        raw: *mut raw::LIBSSH2_CHANNEL,
+        sess: &Rc<SessionInner>,
+    ) -> Result<Self, Error> {
+        if raw.is_null() {
+            Err(Error::last_error_raw(sess.raw).unwrap_or_else(Error::unknown))
+        } else {
+            Ok(Self {
+                raw,
+                sess: Rc::clone(sess),
+                read_limit: None,
+            })
+        }
+    }
+}
+
 /// A channel can have a number of streams, each identified by an id, each of
 /// which implements the `Read` and `Write` traits.
-pub struct Stream<'channel, 'sess: 'channel> {
-    channel: &'channel mut Channel<'sess>,
+pub struct Stream<'channel> {
+    channel: &'channel mut Channel,
     id: i32,
 }
 
@@ -61,7 +78,7 @@ pub struct WriteWindow {
     pub window_size_initial: u32,
 }
 
-impl<'sess> Channel<'sess> {
+impl Channel {
     /// Set an environment variable in the remote channel's process space.
     ///
     /// Note that this does not make sense for all channel types and may be
@@ -187,7 +204,7 @@ impl<'sess> Channel<'sess> {
     /// Get a handle to the stderr stream of this channel.
     ///
     /// The returned handle implements the `Read` and `Write` traits.
-    pub fn stderr<'a>(&'a mut self) -> Stream<'a, 'sess> {
+    pub fn stderr<'a>(&'a mut self) -> Stream<'a> {
         self.stream(::EXTENDED_DATA_STDERR)
     }
 
@@ -200,7 +217,7 @@ impl<'sess> Channel<'sess> {
     ///
     /// * FLUSH_EXTENDED_DATA - Flush all extended data substreams
     /// * FLUSH_ALL - Flush all substreams
-    pub fn stream<'a>(&'a mut self, stream_id: i32) -> Stream<'a, 'sess> {
+    pub fn stream<'a>(&'a mut self, stream_id: i32) -> Stream<'a> {
         Stream {
             channel: self,
             id: stream_id,
@@ -252,7 +269,7 @@ impl<'sess> Channel<'sess> {
             }
             let slice = slice::from_raw_parts(ptr as *const u8, len as usize);
             let ret = slice.to_vec();
-            raw::libssh2_free(chan.sess.raw(), ptr as *mut c_void);
+            raw::libssh2_free(chan.sess.raw, ptr as *mut c_void);
             String::from_utf8(ret).ok()
         }
     }
@@ -351,22 +368,7 @@ impl<'sess> Channel<'sess> {
     }
 }
 
-impl<'sess> SessionBinding<'sess> for Channel<'sess> {
-    type Raw = raw::LIBSSH2_CHANNEL;
-
-    unsafe fn from_raw(sess: &'sess Session, raw: *mut raw::LIBSSH2_CHANNEL) -> Channel<'sess> {
-        Channel {
-            raw: raw,
-            sess: sess,
-            read_limit: None,
-        }
-    }
-    fn raw(&self) -> *mut raw::LIBSSH2_CHANNEL {
-        self.raw
-    }
-}
-
-impl<'sess> Write for Channel<'sess> {
+impl Write for Channel {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
         self.stream(0).write(buf)
     }
@@ -376,13 +378,13 @@ impl<'sess> Write for Channel<'sess> {
     }
 }
 
-impl<'sess> Read for Channel<'sess> {
+impl Read for Channel {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
         self.stream(0).read(buf)
     }
 }
 
-impl<'sess> Drop for Channel<'sess> {
+impl Drop for Channel {
     fn drop(&mut self) {
         unsafe {
             let _ = raw::libssh2_channel_free(self.raw);
@@ -390,7 +392,7 @@ impl<'sess> Drop for Channel<'sess> {
     }
 }
 
-impl<'channel, 'sess> Read for Stream<'channel, 'sess> {
+impl<'channel> Read for Stream<'channel> {
     fn read(&mut self, data: &mut [u8]) -> io::Result<usize> {
         if self.channel.eof() {
             return Ok(0);
@@ -424,7 +426,7 @@ impl<'channel, 'sess> Read for Stream<'channel, 'sess> {
     }
 }
 
-impl<'channel, 'sess> Write for Stream<'channel, 'sess> {
+impl<'channel> Write for Stream<'channel> {
     fn write(&mut self, data: &[u8]) -> io::Result<usize> {
         unsafe {
             let rc = raw::libssh2_channel_write_ex(
diff --git a/src/error.rs b/src/error.rs
index 5eb92f9..d5dd397 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -15,14 +15,12 @@ pub struct Error {
 }
 
 impl Error {
-    /// Generate the last error that occurred for a `Session`.
-    ///
-    /// Returns `None` if there was no last error.
-    pub fn last_error(sess: &Session) -> Option<Error> {
+    #[doc(hidden)]
+    pub fn last_error_raw(raw: *mut raw::LIBSSH2_SESSION) -> Option<Error> {
         static STATIC: () = ();
         unsafe {
             let mut msg = 0 as *mut _;
-            let rc = raw::libssh2_session_last_error(sess.raw(), &mut msg, 0 as *mut _, 0);
+            let rc = raw::libssh2_session_last_error(raw, &mut msg, 0 as *mut _, 0);
             if rc == 0 {
                 return None;
             }
@@ -31,6 +29,13 @@ impl Error {
         }
     }
 
+    /// Generate the last error that occurred for a `Session`.
+    ///
+    /// Returns `None` if there was no last error.
+    pub fn last_error(sess: &Session) -> Option<Error> {
+        Self::last_error_raw(sess.raw())
+    }
+
     /// Create a new error for the given code and message
     pub fn new(code: libc::c_int, msg: &'static str) -> Error {
         Error {
diff --git a/src/lib.rs b/src/lib.rs
index b02b77d..64d5601 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -141,6 +141,7 @@ pub use channel::{Channel, ExitSignal, ReadWindow, Stream, WriteWindow};
 pub use error::Error;
 pub use knownhosts::{Host, Hosts, KnownHosts};
 pub use listener::Listener;
+use session::SessionInner;
 pub use session::{ScpFileStat, Session};
 pub use sftp::{File, FileStat, FileType, OpenType};
 pub use sftp::{OpenFlags, RenameFlags, Sftp};
diff --git a/src/listener.rs b/src/listener.rs
index 33a8d5c..229bb78 100644
--- a/src/listener.rs
+++ b/src/listener.rs
@@ -1,40 +1,40 @@
-use util::SessionBinding;
-use {raw, Channel, Error, Session};
+use std::rc::Rc;
+use {raw, Channel, Error, SessionInner};
 
 /// A listener represents a forwarding port from the remote server.
 ///
 /// New channels can be accepted from a listener which represent connections on
 /// the remote server's port.
-pub struct Listener<'sess> {
+pub struct Listener {
     raw: *mut raw::LIBSSH2_LISTENER,
-    sess: &'sess Session,
+    sess: Rc<SessionInner>,
 }
 
-impl<'sess> Listener<'sess> {
+impl Listener {
     /// Accept a queued connection from this listener.
-    pub fn accept(&mut self) -> Result<Channel<'sess>, Error> {
+    pub fn accept(&mut self) -> Result<Channel, Error> {
         unsafe {
-            let ret = raw::libssh2_channel_forward_accept(self.raw);
-            SessionBinding::from_raw_opt(self.sess, ret)
+            let chan = raw::libssh2_channel_forward_accept(self.raw);
+            Channel::from_raw_opt(chan, &self.sess)
         }
     }
-}
-
-impl<'sess> SessionBinding<'sess> for Listener<'sess> {
-    type Raw = raw::LIBSSH2_LISTENER;
 
-    unsafe fn from_raw(sess: &'sess Session, raw: *mut raw::LIBSSH2_LISTENER) -> Listener<'sess> {
-        Listener {
-            raw: raw,
-            sess: sess,
+    pub(crate) fn from_raw_opt(
+        raw: *mut raw::LIBSSH2_LISTENER,
+        sess: &Rc<SessionInner>,
+    ) -> Result<Self, Error> {
+        if raw.is_null() {
+            Err(Error::last_error_raw(sess.raw).unwrap_or_else(Error::unknown))
+        } else {
+            Ok(Self {
+                raw,
+                sess: Rc::clone(sess),
+            })
         }
     }
-    fn raw(&self) -> *mut raw::LIBSSH2_LISTENER {
-        self.raw
-    }
 }
 
-impl<'sess> Drop for Listener<'sess> {
+impl Drop for Listener {
     fn drop(&mut self) {
         unsafe {
             let _ = raw::libssh2_channel_forward_cancel(self.raw);
diff --git a/src/session.rs b/src/session.rs
index 117af4f..47329aa 100644
--- a/src/session.rs
+++ b/src/session.rs
@@ -14,8 +14,8 @@ use util::{self, SessionBinding};
 use {raw, ByApplication, DisconnectCode, Error, HostKeyType};
 use {Agent, Channel, HashType, KnownHosts, Listener, MethodType, Sftp};
 
-struct SessionInner {
-    raw: *mut raw::LIBSSH2_SESSION,
+pub(crate) struct SessionInner {
+    pub(crate) raw: *mut raw::LIBSSH2_SESSION,
     tcp: RefCell<Option<TcpStream>>,
 }
 
@@ -485,7 +485,7 @@ impl Session {
                 shost.as_ptr(),
                 sport as c_int,
             );
-            SessionBinding::from_raw_opt(self, ret)
+            Channel::from_raw_opt(ret, &self.inner)
         }
     }
 
@@ -509,7 +509,7 @@ impl Session {
                 &mut bound_port,
                 queue_maxsize.unwrap_or(0) as c_int,
             );
-            SessionBinding::from_raw_opt(self, ret).map(|l| (l, bound_port as u16))
+            Listener::from_raw_opt(ret, &self.inner).map(|l| (l, bound_port as u16))
         }
     }
 
@@ -523,7 +523,7 @@ impl Session {
         unsafe {
             let mut sb: libc::stat = mem::zeroed();
             let ret = raw::libssh2_scp_recv(self.inner.raw, path.as_ptr(), &mut sb);
-            let mut c: Channel = try!(SessionBinding::from_raw_opt(self, ret));
+            let mut c = Channel::from_raw_opt(ret, &self.inner)?;
 
             // Hm, apparently when we scp_recv() a file the actual channel
             // itself does not respond well to read_to_end(), and it also sends
@@ -561,7 +561,7 @@ impl Session {
                 mtime as libc::time_t,
                 atime as libc::time_t,
             );
-            SessionBinding::from_raw_opt(self, ret)
+            Channel::from_raw_opt(ret, &self.inner)
         }
     }
 
@@ -574,7 +574,7 @@ impl Session {
     pub fn sftp(&self) -> Result<Sftp, Error> {
         unsafe {
             let ret = raw::libssh2_sftp_init(self.inner.raw);
-            SessionBinding::from_raw_opt(self, ret)
+            Sftp::from_raw_opt(ret, &self.inner)
         }
     }
 
@@ -603,7 +603,7 @@ impl Session {
                     .unwrap_or(0 as *const _) as *const _,
                 message_len as c_uint,
             );
-            SessionBinding::from_raw_opt(self, ret)
+            Channel::from_raw_opt(ret, &self.inner)
         }
     }
 
@@ -735,6 +735,20 @@ impl Session {
     }
 }
 
+impl SessionInner {
+    /// Translate a return code into a Rust-`Result`.
+    pub fn rc(&self, rc: c_int) -> Result<(), Error> {
+        if rc >= 0 {
+            Ok(())
+        } else {
+            match Error::last_error_raw(self.raw) {
+                Some(e) => Err(e),
+                None => Ok(()),
+            }
+        }
+    }
+}
+
 impl Drop for SessionInner {
     fn drop(&mut self) {
         unsafe {
diff --git a/src/sftp.rs b/src/sftp.rs
index a02c4db..37f8dfd 100644
--- a/src/sftp.rs
+++ b/src/sftp.rs
@@ -1,19 +1,19 @@
 use libc::{c_int, c_long, c_uint, c_ulong, size_t};
 use std::io::prelude::*;
 use std::io::{self, ErrorKind, SeekFrom};
-use std::marker;
 use std::mem;
 use std::path::{Path, PathBuf};
+use std::rc::Rc;
 
-use util::{self, SessionBinding};
-use {raw, Channel, Error, Session};
+use util;
+use {raw, Error, SessionInner};
 
 /// A handle to a remote filesystem over SFTP.
 ///
 /// Instances are created through the `sftp` method on a `Session`.
-pub struct Sftp<'sess> {
+pub struct Sftp {
     raw: *mut raw::LIBSSH2_SFTP,
-    _marker: marker::PhantomData<Channel<'sess>>,
+    _sess: Rc<SessionInner>,
 }
 
 /// A file handle to an SFTP connection.
@@ -25,7 +25,7 @@ pub struct Sftp<'sess> {
 /// of `Sftp`.
 pub struct File<'sftp> {
     raw: *mut raw::LIBSSH2_SFTP_HANDLE,
-    sftp: &'sftp Sftp<'sftp>,
+    sftp: &'sftp Sftp,
 }
 
 /// Metadata information about a remote file.
@@ -102,7 +102,21 @@ pub enum OpenType {
     Dir = raw::LIBSSH2_SFTP_OPENDIR as isize,
 }
 
-impl<'sess> Sftp<'sess> {
+impl Sftp {
+    pub(crate) fn from_raw_opt(
+        raw: *mut raw::LIBSSH2_SFTP,
+        sess: &Rc<SessionInner>,
+    ) -> Result<Self, Error> {
+        if raw.is_null() {
+            Err(Error::last_error_raw(sess.raw).unwrap_or_else(Error::unknown))
+        } else {
+            Ok(Self {
+                raw,
+                _sess: Rc::clone(sess),
+            })
+        }
+    }
+
     /// Open a handle to a file.
     pub fn open_mode(
         &self,
@@ -355,21 +369,7 @@ impl<'sess> Sftp<'sess> {
     }
 }
 
-impl<'sess> SessionBinding<'sess> for Sftp<'sess> {
-    type Raw = raw::LIBSSH2_SFTP;
-
-    unsafe fn from_raw(_sess: &'sess Session, raw: *mut raw::LIBSSH2_SFTP) -> Sftp<'sess> {
-        Sftp {
-            raw: raw,
-            _marker: marker::PhantomData,
-        }
-    }
-    fn raw(&self) -> *mut raw::LIBSSH2_SFTP {
-        self.raw
-    }
-}
-
-impl<'sess> Drop for Sftp<'sess> {
+impl Drop for Sftp {
     fn drop(&mut self) {
         unsafe { assert_eq!(raw::libssh2_sftp_shutdown(self.raw), 0) }
     }
@@ -380,10 +380,7 @@ impl<'sftp> File<'sftp> {
     /// given session.
     ///
     /// This consumes ownership of `raw`.
-    unsafe fn from_raw(
-        sftp: &'sftp Sftp<'sftp>,
-        raw: *mut raw::LIBSSH2_SFTP_HANDLE,
-    ) -> File<'sftp> {
+    unsafe fn from_raw(sftp: &'sftp Sftp, raw: *mut raw::LIBSSH2_SFTP_HANDLE) -> File<'sftp> {
         File {
             raw: raw,
             sftp: sftp,