Rip out the session pooling.

It was open to accidentally prematurely closing sessions just because
they happened to connect to the same host, port, etc.

While we're at it, make the fourth arugment to open mandatory.

Fixes: #6

Author: conormcd

src/clj_libssh2/session.clj

@@ -1,16 +1,18 @@
 (ns clj-libssh2.session
   "Functions for creating and managing sessions."
-  (:require [clj-libssh2.libssh2 :as libssh2]
+  (:require [clojure.set :as set]
+            [clj-libssh2.libssh2 :as libssh2]
             [clj-libssh2.libssh2.session :as libssh2-session]
             [clj-libssh2.authentication :refer [authenticate]]
             [clj-libssh2.error :refer [handle-errors with-timeout]]
             [clj-libssh2.known-hosts :as known-hosts]
             [clj-libssh2.socket :as socket]))
 
 (def sessions
-  "A pool of currently running sessions. This is an atomic map where the keys
-   are session IDs and the values are running session objects."
-  (atom {}))
+  "An atomic set of currently active sessions. This is used to trigger calls to
+   libssh2/init and libssh2/exit at appropriate times. It's also used to
+   protect against attempting to close sessions twice."
+  (atom #{}))
 
 (def default-opts
   "The default options for a session. These are not only the defaults, but an
@@ -23,7 +25,7 @@
    :read-timeout 60000
    :write-chunk-size (* 1024 1024)})
 
-(defrecord Session [id session socket host port options])
+(defrecord Session [session socket host port options])
 
 (defn- create-session
   "Make a native libssh2 session object.
@@ -96,27 +98,6 @@
     (with-timeout :session
       (libssh2-session/handshake session socket))))
 
-(defn- session-id
-  "Generate the session ID that will be used to pool sessions.
-
-   Arguments:
-
-   host         The hostname or IP of the remote host.
-   port         The port we're connecting to.
-   credentials  The credentials to be used to authenticate the session.
-   opts         The session options.
-
-   Return:
-
-   A string that will uniquely identify a session."
-  [host port credentials opts]
-  (format "%s@%s:%d-%d-%d"
-          (:username credentials)
-          host
-          port
-          (.hashCode credentials)
-          (.hashCode opts)))
-
 (defn close
   "Disconnect an SSH session and discard the session.
 
@@ -127,11 +108,11 @@
    Return:
 
    nil."
-  [{id :id}]
-  (when-let [session (get @sessions id)]
+  [session]
+  (when (contains? @sessions session)
     (destroy-session session)
     (socket/close (:socket session))
-    (swap! sessions dissoc id)
+    (swap! sessions set/difference #{session})
     (when (empty? @sessions)
       (libssh2/exit))))
 
@@ -149,33 +130,26 @@
    Return:
 
    A Session object for the connected and authenticated session."
-  ([host port credentials]
-   (open host port credentials {}))
-  ([host port credentials opts]
-   (when (empty? @sessions)
-     (handle-errors nil (libssh2/init 0)))
-   (let [session-options (create-session-options opts)
-         id (session-id host port credentials session-options)]
-     (if (contains? @sessions id)
-       (get @sessions id)
-       (let [session (Session. id
-                               (create-session)
-                               (socket/connect host port)
-                               host
-                               port
-                               session-options)]
-         (when (> 0 (:socket session))
-           (destroy-session session "Shutting down due to bad socket." true))
-         (try
-           (libssh2-session/set-blocking (:session session) 0)
-           (handshake session)
-           (known-hosts/check session)
-           (authenticate session credentials)
-           (swap! sessions assoc (:id session) session)
-           (get @sessions (:id session))
-           (catch Exception e
-             (close session)
-             (throw e))))))))
+  [host port credentials opts]
+  (when (empty? @sessions)
+    (handle-errors nil (libssh2/init 0)))
+  (let [session (Session. (create-session)
+                          (socket/connect host port)
+                          host
+                          port
+                          (create-session-options opts))]
+    (when (> 0 (:socket session))
+      (destroy-session session "Shutting down due to bad socket." true))
+    (try
+      (libssh2-session/set-blocking (:session session) 0)
+      (handshake session)
+      (known-hosts/check session)
+      (authenticate session credentials)
+      (swap! sessions conj session)
+      session
+      (catch Exception e
+        (close session)
+        (throw e)))))
 
 (defmacro with-session
   "A convenience macro for running some code with a particular session.

test/clj_libssh2/test_agent.clj

@@ -9,9 +9,7 @@
 
 (defn agent-session
   []
-  (session/open test/ssh-host
-                test/ssh-port
-                {:username (test/ssh-user)}))
+  (session/open test/ssh-host test/ssh-port {:username (test/ssh-user)} {}))
 
 (defn open-and-close
   []

test/clj_libssh2/test_authentication.clj

@@ -11,7 +11,7 @@
 (defn auth
   [creds]
   (is (= 0 (count @session/sessions)))
-  (let [session (session/open test/ssh-host test/ssh-port creds)]
+  (let [session (session/open test/ssh-host test/ssh-port creds {})]
     (is (= 1 (count @session/sessions)))
     (session/close session))
   (is (= 0 (count @session/sessions))))

test/clj_libssh2/test_session.clj

@@ -12,7 +12,8 @@
   (session/open test/ssh-host
                 test/ssh-port
                 (merge {:username (test/ssh-user)}
-                       (apply hash-map creds))))
+                       (apply hash-map creds))
+                {}))
 
 (deftest close-is-robust
   (testing "Closing a good, open connection does not fail."
@@ -31,16 +32,6 @@
     (session/close nil)))
 
 (deftest open-works
-  (testing "sessions are pooled"
-    (is (= 0 (count @session/sessions)))
-    (let [session1 (open)]
-      (is (= 1 (count @session/sessions)))
-      (let [session2 (open)]
-        (is (= 1 (count @session/sessions)))
-        (is (= session1 session2))
-        (session/close session1)
-        (session/close session2)
-        (is (= 0 (count @session/sessions))))))
   (testing "throws but doesn't crash on handshake failure"
     (with-redefs [libssh2-session/handshake (constantly libssh2/ERROR_PROTO)]
       (is (= 0 (count @session/sessions)))
@@ -56,5 +47,4 @@
                                               test/ssh-port
                                               {:username (test/ssh-user)}
                                               {:bad-option :bad-option-value})))
-    (is (= 0 (count @session/sessions)))
-    ))
+    (is (= 0 (count @session/sessions)))))