Fix GC'ing of SSH objects.

The program doesn't crashes anymore during GC'ing of channels and
keys.

* src/session-type.c (free_session): Mark all related channels as
  freed.
  (guile_ssh_make_session): Initialize the channels array.
* src/key-type.c (mark_key_smob): Fix smob marking.
* src/channel-type.c (free_channel): Check if the channel has been
  already freed along with the related SSH session.
  (guile_ssh_make_channel): Store the reference to the channel in
  the array of channels related to the SSH session.
* src/channel-type.h: Add is_channel_alive field to channel_data.
* TODO: Add to the repository.

7 files changed, 78 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 29b4378..3e082ab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2013-07-13  Artyom Poptsov  <poptsov.artyom@gmail.com>
+
+	Fix GC'ing of SSH objects: the program doesn't crashes anymore
+	during GC'ing of channels and keys.
+	* src/session-type.c (free_session): Mark all related channels as
+	freed.
+	(guile_ssh_make_session): Initialize the channels array.
+	* src/key-type.c (mark_key_smob): Fix smob marking.
+	* src/channel-type.c (free_channel): Check if the channel has been
+	already freed along with the related SSH session.
+	(guile_ssh_make_channel): Store the reference to the channel in
+	the array of channels related to the SSH session.
+	* src/channel-type.h: Add is_channel_alive field to channel_data.
+	* TODO: Add to the repository.
+
 2013-06-23  Artyom Poptsov  <poptsov.artyom@gmail.com>
 
 	* src/key-type.h: Improve storing of public keys represented as a
diff --git a/TODO b/TODO
new file mode 100644
index 0000000..58684be
--- /dev/null
+++ b/TODO
@@ -0,0 +1,9 @@
+-*- Mode: Outline -*-
+
+* Improve GC'ing of SSH channels.
+  Array of channel references stored in session_data can only grow for
+  now.  Generally it means two things: a) the maximum number of
+  channels is limited by UINT32_MAX, and b) closed channels won't be
+  removed from the array.  That is, if your program constantly opens
+  new channels for one session, then at some point you can run into
+  the problems with making new channels.
diff --git a/src/channel-type.c b/src/channel-type.c
index 75e69a5..acd3a06 100644
--- a/src/channel-type.c
+++ b/src/channel-type.c
@@ -42,7 +42,15 @@ free_channel (SCM channel_smob)
   struct channel_data *data
     = (struct channel_data *) SCM_SMOB_DATA (channel_smob);
 
-  ssh_channel_free (data->ssh_channel);
+  /* If the SSH session is already freed, we don't need to call
+     ssh_channel_free for the channel, because allocated resourses are
+     already freed by ssh_free.  Moreover, we'll get segmentation
+     fault if we try to call ssh_channel_free when the session is
+     GC'ed. */
+  if (data->is_session_alive)
+    ssh_channel_free (data->ssh_channel);
+
+  SCM_SET_SMOB_DATA (channel_smob, NULL);
 
   return 0;
 }
@@ -52,6 +60,9 @@ SCM
 guile_ssh_make_channel (SCM session_smob)
 {
   SCM smob;
+  size_t cnt;                   /* Channel count */
+  size_t old_sz;                /* Old channels array size */
+  size_t new_sz;                /* New channels array size */
 
   struct session_data *session_data
     = (struct session_data *) SCM_SMOB_DATA (session_smob);
@@ -63,8 +74,25 @@ guile_ssh_make_channel (SCM session_smob)
   if (channel_data->ssh_channel == NULL)
     return SCM_BOOL_F;
 
+  channel_data->is_session_alive = 1;
+
   SCM_NEWSMOB (smob, channel_tag, channel_data);
 
+  /* FIXME: channels array can only grow for now.  Generally it means
+     two things: a) the maximum number of channels is limited by
+     UINT32_MAX, and b) closed channels won't be removed from the
+     array. */
+  cnt    = session_data->channel_cnt;
+  old_sz = sizeof (struct channel_data *) * cnt;
+  new_sz = sizeof (struct channel_data *) * (cnt + 1);
+  session_data->channels = scm_gc_realloc (session_data->channels,
+                                           old_sz, new_sz,
+                                           "channel list");
+
+  ++session_data->channel_cnt;
+
+  session_data->channels[session_data->channel_cnt - 1] = channel_data;
+
   return smob;
 }
 
diff --git a/src/channel-type.h b/src/channel-type.h
index 286e451..835cfcc 100644
--- a/src/channel-type.h
+++ b/src/channel-type.h
@@ -28,6 +28,7 @@ extern scm_t_bits channel_tag;
 /* Smob data. */
 struct channel_data {
   ssh_channel ssh_channel;
+  int is_session_alive;
 };
 
 
diff --git a/src/key-type.c b/src/key-type.c
index 1072111..3510c17 100644
--- a/src/key-type.c
+++ b/src/key-type.c
@@ -27,9 +27,9 @@ scm_t_bits key_tag; /* Smob tag. */
 
 /* Smob marking */
 SCM
-scm_markcdr (SCM key_smob)
+mark_key_smob (SCM key_smob)
 {
-  return SCM_SMOB_OBJECT (key_smob);
+  return SCM_BOOL_F;
 }
 
 /* Free the smob. */
@@ -37,7 +37,6 @@ size_t
 free_key_smob (SCM key_smob)
 {
   struct key_data *data;
-
   scm_assert_smob_type (key_tag, key_smob);
 
   data = (struct key_data *) SCM_SMOB_DATA (key_smob);
@@ -169,7 +168,7 @@ void
 init_key_type (void)
 {
   key_tag = scm_make_smob_type ("ssh:key", sizeof (struct key_data));
-  scm_set_smob_mark (key_tag, scm_markcdr);
+  scm_set_smob_mark (key_tag, mark_key_smob);
   scm_set_smob_free (key_tag, free_key_smob);
 
   scm_c_define_gsubr ("ssh:key?",         1, 0, 0, guile_ssh_is_key_p);
diff --git a/src/session-type.c b/src/session-type.c
index 6d72281..5fd8082 100644
--- a/src/session-type.c
+++ b/src/session-type.c
@@ -23,6 +23,7 @@
 #include <string.h>
 
 #include "session-type.h"
+#include "channel-type.h"
 #include "error.h"
 
 #define PRINT_DEBUG(data)\
@@ -40,11 +41,21 @@ mark_session (SCM session_smob)
 size_t
 free_session (SCM session_smob)
 {
+  size_t i;
   struct session_data *data
     = (struct session_data *) SCM_SMOB_DATA (session_smob);
 
   ssh_disconnect (data->ssh_session);
   ssh_free (data->ssh_session);
+
+  for (i = 0; i < data->channel_cnt; ++i)
+    data->channels[i]->is_session_alive = 0;
+
+  if (data->channel_cnt)
+    scm_gc_free (data->channels, sizeof data->channels, "channel list");
+
+  SCM_SET_SMOB_DATA (session_smob, NULL);
+
   return 0;
 }
 
@@ -62,6 +73,9 @@ guile_ssh_make_session (void)
   if (session_data->ssh_session == NULL)
     return SCM_BOOL_F;
 
+  session_data->channels = NULL;
+  session_data->channel_cnt = 0;
+
   SCM_NEWSMOB (smob, session_tag, session_data);
 
   return smob;
diff --git a/src/session-type.h b/src/session-type.h
index a48fd11..81aa2fc 100644
--- a/src/session-type.h
+++ b/src/session-type.h
@@ -21,12 +21,19 @@
 
 #include <libguile.h>
 #include <libssh/libssh.h>
+#include "channel-type.h"
 
 extern scm_t_bits session_tag;
 
 
 struct session_data {
   ssh_session ssh_session;
+
+  /* The reason for storing references to all channels related to the
+     session is that we have to prevent freeing of the channels that
+     are normally freed along with the session. */
+  size_t channel_cnt;
+  struct channel_data **channels;
 };