[Ecls-list] "socket crash bug" im Vim+ECL fixed

[Larry Clapp]

[ Note: Cross-posted to both the Slim-Vim list and the ECL devel list. ]

I fixed the "socket crash bug" in Vim+ECL, and I think I discovered
what caused it (in that order!).

The fix: Change vim:add-input-listener so that instead of evaling a
list, it funcalls a closure.

Changes:
  if_ecl.lisp:
     (defun add-input-listener (stream callback)
       "Registers a callback to be invoked when data arrives to a stream"
       (check-type stream stream)
       (check-type callback function)
       (setf (gethash stream callbacks) callback)
  -    (add-input-listener-int stream `(safe-eval '(cl:funcall ,callback) nil))
  +    (add-input-listener-int stream callback)
       t)

  if_ecl.c:
   static void listener_callback(int fd, void *data)
   {
  -    safe_eval_form((cl_object)data, FALSE);
  +    cl_funcall(1, (cl_object) data);

I didn't make this change to fix the bug; I only made it because I
didn't like that invoking the network callback went like this

  vim
  -> listener_callback
  -> safe_eval_form
  -> si_eval_with_env
  -> SAFE-EVAL
  -> SAFE-EVAL
  -> the callback

and I wanted it more like

  vim
  -> listener_callback
  -> the callback

So the patch changed the invocation sequence, which made the bug
either go away, or made it a lot rarer.  I assumed that using
SAFE-EVAL somehow caused problems, and I wanted to know why, so I
added it back into the callback.  It didn't fail.  I added another.
It still didn't fail.  Somewhere in there, I re-read Jim's recent post
and studied what I changed, and what I changed it from, and gradually
the light came on.

Jim's post, in part:

  > The GC ECL uses is the Boehm GC, which (thankfully) doesn't move
  > things around. You do have to be careful of storing pointers to
  > gc-allocated memory in non gc-allocated memory, e.g. storing an
  > ECL closure pointer in malloc'd memory, such things arn't detected
  > by the mark and sweep and can lead to premature collection and
  > horrible problems. This is why the callbacks are stored in a CL
  > hash-table as well to stop them from disappearing. Of course I may
  > have got this wrong and messed up really really badly, maybe it is
  > worth turning the gc off temporarily just to see what happens.

Look closely at ADD-INPUT-LISTENER, above.  Notice that Jim puts the
callback into a hash table (so some Lisp datastructure will have a
reference to it, so the GC won't collect it), and then wraps a list
around it and gives Vim a pointer to the list, which we later EVAL.
(Now see there?  I've just about given it away!  :)  The *callback* is
in the hashtable, but the *list around it isn't*!  And nothing else
references it, either!  One GC later and it's game over.

Anyway, the patch (all two lines of it :) is at theclapp.org[1].
Happy hacking!

-- Larry


[1] http://theclapp.org/repos/vim70+async+ecl/