Subject: [clisp-list] socket and pipe streams leak handles if not explicitly closed

From: Todd Sabin <[email-masked]>
Date: Thu Jan 10 20:21:07 2002

If I do

(defun foo ()
  (socket-connect 80 "www.yahoo.com")
  (socket-server 8000)
  (values))

(foo)

and then 

(gc)

the socket server handle is closed, but the client socket handle is
not.  The same is true for streams obtained from socket-accept and all
flavors of pipe streams.  Unless they are explicitly closed, the
handles are lost during gc.

The problem is that gc only calls builtin_stream_close for streams
that are on the open_files list, and none of the above are put on it
when created.  Below is a possible patch which appears to resolve this
for me.  (I'm not sure about the STACK change to make_printer_stream,
but I think it's right.)

In the process of figuring this out, I also noticed that streams are
only removed from the open_files list if they happen to be buffered.
The patch also fixes that.

Todd

diff -u -r1.246 stream.d
--- stream.d	2002/01/07 21:41:36	1.246
+++ stream.d	2002/01/11 02:57:47
@@ -5808,6 +5808,18 @@
   return stream;
 }
 
+# UP: add a stream to the list of open streams (open_files)
+# add_to_open_streams()
+# > stream
+# can trigger GC
+local object add_to_open_streams (object stream) {
+  pushSTACK(stream);
+  var object new_cons = allocate_cons();
+  Car(new_cons) = stream = popSTACK();
+  Cdr(new_cons) = O(open_files);
+  O(open_files) = new_cons;
+  return stream;
+}
 
 # File-Stream
 # ===========
@@ -7667,13 +7679,8 @@
   }
   skipSTACK(3);
   # extend List of open File-Streams by stream:
-  pushSTACK(stream);
-  {
-    var object new_cons = allocate_cons();
-    Car(new_cons) = stream = popSTACK();
-    Cdr(new_cons) = O(open_files);
-    O(open_files) = new_cons;
-  }
+  stream = add_to_open_streams(stream);
+
   # treat Mode :APPEND:
   # CLHS says that :APPEND implies that "the file pointer is _initially_
   # positioned at the end of the file". Note that this is different from
@@ -7876,8 +7883,6 @@
   ChannelStream_fini(stream);
   # make Components invalid (close_dummys comes later):
   closed_buffered(stream);
-  # remove stream from the List of all open File-Streams:
-  O(open_files) = deleteq(O(open_files),stream);
 }
 
 # (SYS::FILE-STREAM-P stream) == (TYPEP stream 'FILE-STREAM)
@@ -10467,16 +10472,7 @@
 # Returns an interactive Terminal-Stream.
 # can trigger GC
 local object make_terminal_stream (void) {
-  var object stream = make_terminal_stream_();
-  # extend List of open Streams by stream:
-  pushSTACK(stream);
-  {
-    var object new_cons = allocate_cons();
-    Car(new_cons) = stream = popSTACK();
-    Cdr(new_cons) = O(open_files);
-    O(open_files) = new_cons;
-  }
-  return stream;
+  return add_to_open_streams (make_terminal_stream_ ());
 }

@@ -13381,7 +13377,6 @@
 # UP: Returns a Printer-Stream.
 # can trigger GC
 local object make_printer_stream (void) {
-  pushSTACK(allocate_cons()); # Cons for List
   pushSTACK(allocate_handle(Handle_NULL)); # Handle-Wrapping
   var object stream = # new Stream, only WRITE-CHAR allowed
     allocate_stream(strmflags_wr_ch_B,strmtype_printer,strm_len+1,0);
@@ -13398,12 +13393,7 @@
   TheStream(stream)->strm_wr_ch_array = P(wr_ch_array_dummy);
   TheStream(stream)->strm_printer_handle = popSTACK();
   # extend List of open Streams by stream:
-  {
-    var object new_cons = popSTACK();
-    Car(new_cons) = stream;
-    Cdr(new_cons) = O(open_files);
-    O(open_files) = new_cons;
-  }
+  stream = add_to_open_streams (stream);
   clr_break_sem_4();
   return stream;
 }
@@ -13638,7 +13628,7 @@
   ChannelStreamLow_close(stream) = &low_close_pipe;
   TheStream(stream)->strm_pipe_pid = popSTACK(); # Child-Pid
   skipSTACK(4);
-  value1 = stream; mv_count=1; # stream as value
+  value1 = add_to_open_streams (stream); mv_count=1; # stream as value
 }

@@ -13851,7 +13841,7 @@
   ChannelStreamLow_close(stream) = &low_close_pipe;
   TheStream(stream)->strm_pipe_pid = popSTACK(); # Child-Pid
   skipSTACK(4);
-  value1 = stream; mv_count=1; # stream as value
+  value1 = add_to_open_streams (stream); mv_count=1; # stream as value
 }
 
 #ifdef PIPES2
@@ -14039,7 +14029,7 @@
     }
     ChannelStreamLow_close(stream) = &low_close_pipe;
     TheStream(stream)->strm_pipe_pid = STACK_2; # Child-Pid
-    STACK_1 = stream;
+    STACK_1 = add_to_open_streams (stream);
   }
   # allocate Output-Stream:
   {
@@ -14058,7 +14048,7 @@
     }
     ChannelStreamLow_close(stream) = &low_close_pipe;
     TheStream(stream)->strm_pipe_pid = STACK_2; # Child-Pid
-    STACK_0 = stream;
+    STACK_0 = add_to_open_streams (stream);
   }
  #ifdef EMUNIX
   # combine both pipes, for frictionless close:
@@ -14708,8 +14698,9 @@
   value1 = make_socket_stream(handle,&eltype,buffered,
                               TheSocketServer(STACK_3)->host,
                               TheSocketServer(STACK_3)->port);
-  mv_count = 1;
   skipSTACK(4);
+  value1 = add_to_open_streams (value1);
+  mv_count = 1;
 }
 
 local struct timeval * sec_usec (object sec, object usec, struct timeval *tv) {
@@ -14800,6 +14791,8 @@
                               asciz_to_string(hostname,O(misc_encoding)),
                               STACK_4);
   skipSTACK(5);
+  # extend List of open Streams by stream:
+  value1 = add_to_open_streams (value1);
   mv_count = 1;
 }
 
@@ -15949,6 +15942,8 @@
         else
           close_ichannel(stream);
       }
+      # remove stream from the List of all open File-Streams:
+      O(open_files) = deleteq(O(open_files),stream);
       break;
     #ifdef SOCKET_STREAMS
     case strmtype_twoway_socket:

---

From: Sam Steingold <[email-masked]>
Date: Fri Jan 11 09:05:02 2002

> * In message <[email-masked]>
> * On the subject of "[clisp-list] socket and pipe streams leak handles if not explicitly closed"
> * Sent on 10 Jan 2002 23:19:01 -0500
> * Honorable Todd Sabin <[email-masked]> writes:
>

Thanks for the bug report and the patch.
I committed a it with minor changes.
It appears to fix the bug.

-- 
Sam Steingold (http://www.podval.org/~sds)
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
If you want it done right, you have to do it yourself