Re: leak in make-network-process

[Dan Nicolaescu <dann@ics.uci.edu>]

Dan Nicolaescu <dann@ics.uci.edu> writes:

  > Do 
  > 
  > emacs -Q --daemon
  > (to make sure there's a  server running)
  > 
  > then 
  > 
  > emacs -Q -l test.el 
  > 
  > 
  > cat test.el
  > 
  > (defun gc-output (arg)
  >   (interactive "M")
  >   (with-current-buffer "*scratch*"
  >     (insert (format "%s %S\n" arg
  > 		    (garbage-collect)))))
  > 
  > (defun server-running-p (&optional name)
  >   "Test whether server NAME is running.
  > 
  > Return values:
  >   nil              the server is definitely not running.
  >   t                the server seems to be running.
  >   something else   we cannot determine whether it's running without using
  >                    commands which may have to wait for a long time."
  >   (interactive
  >    (list (if current-prefix-arg
  > 	     (read-string "Server name: " nil nil server-name))))
  >   (unless name (setq name server-name))
  >   (condition-case nil
  >       (if server-use-tcp
  > 	  (with-temp-buffer
  > 	    (insert-file-contents-literally (expand-file-name name server-auth-dir))
  > 	    (or (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
  > 		     (assq 'comm
  > 			   (system-process-attributes
  > 			    (string-to-number (match-string 1))))
  > 		     t)
  > 		:other))
  > 	(gc-output "B make-net")
  > 	(delete-process
  > 	 (make-network-process
  > 	  :name "server-client-test" :family 'local :server nil :noquery t
  > 	  :service (expand-file-name name server-socket-dir)))
  > 	(gc-output "A make-net")
  > 	t)
  >     (file-error nil)))
  > 
  > 
  > Now do
  > M-x server-start RET
  > 
  > it won't start a server, but in the it will print in the *scratch*
  > buffer the GC results before and after the `(delete-process (make-network-process ' call.
  > There's a leak of 22 conses.
  > 
  > Adding this at the end of process.c:remove_process:
  > 
  >   {
  >     struct Lisp_Process *p = XPROCESS (proc);
  >     /* Set all Lisp_Object members of struct Lisp_Process to nil.  */
  >     p->tty_name = Qnil;
  >     p->name = Qnil;
  >     p->command = Qnil;
  >     p->filter = Qnil;
  >     p->sentinel = Qnil;
  >     p->log = Qnil;
  >     p->buffer = Qnil;
  >     p->childp = Qnil;
  >     p->plist = Qnil;
  >     p->type = Qnil;
  >     p->mark = Qnil;
  >     p->status = Qnil;
  >     p->decode_coding_system = Qnil;
  >     p->decoding_buf = Qnil;
  >     p->encode_coding_system = Qnil;
  >     p->encoding_buf = Qnil;
  >   }
  > 
  > reduces the leak to 4 conses.  Not sure if doing that is TRTD...

Actually only this is needed:

  {
    struct Lisp_Process *p = XPROCESS (proc);
    /* Set all Lisp_Object members of struct Lisp_Process to nil.  */
    p->childp = Qnil;
    p->status = Qnil;
  }


p->childp is a plist in the case of network processes
p->status is set to a cons by Fdelete_process for network processes

Why can't GC get these fields?  Should something like this be checked
in?