[Mono-bugs] [Bug 74673][Wis] Changed - [PATCH] New UnixListener and UnixClient classes

[bugzilla-daemon@bugzilla.ximian.com]

Please do not reply to this email- if you want to comment on the bug, go to the
URL shown below and enter your comments there.

Changed by jonpryor@vt.edu.

http://bugzilla.ximian.com/show_bug.cgi?id=74673

--- shadow/74673	2005-04-21 12:32:18.000000000 -0400
+++ shadow/74673.tmp.23370	2005-04-21 13:15:33.000000000 -0400
@@ -44,6 +44,84 @@
 Jon, ok to commit these files and the fix to UnixEndPoint in
 Mono.Unix? Any comment on the behavior in the listener when the file
 already exists?
 
 ------- Additional Comments From joeshaw@novell.com  2005-04-21 12:32 -------
 The Connect() idea sounds like the way to go to me.
+
+------- Additional Comments From jonpryor@vt.edu  2005-04-21 13:15 -------
+Please merge the Mono.Posix UnixEndPoint changes into Mono.Unix.
+
+As for the classes, I have the following comments:
+
+Summary: I'm against permitting derivation unless there's a
+good use case, *especially* for types that contain a
+finalizer.  If you want to permit derivation, split the
+finalizable resource into a separate class.  This is to keep
+the finalizable GC graph as small as possible, improving GC
+performance.  See also: 
+
+http://www.bluebytesoftware.com/blog/PermaLink.aspx?guid=88e62cdf-5919-4ac7-bc33-20c06ae539ae
+
+Though maybe I'm being too nit-picky...
+
+UnixClient:
+	- It should use the Mono.Unix namespace, and be placed 
+	  into Mono.Unix.
+	- The class has no functional virtual methods.  It should 
+	  be `sealed', unless there's a good use case to permit 
+	  derivation.
+	- Dispose(bool) should be non-virtual (if the class 
+	  becomes `sealed').
+	- Dispose(bool) should set `disposed = true` at the end.
+	  (Think exception safety.)
+	- `this.' isn't necessary (see Close()).
+	- I dislike "brace hugging"; e.g. "else" and "finally" 
+	  should start a new line
+	- CheckDisposed() should be called at the start of every
+	  major method/property.  This includes LingerState,
+	  ReceiveBufferSize, etc.  GetStream() calls it in a
+	  finally block (WTF?).
+	- There should be a UnixClient(UnixEndPoint) constructor
+	- Protected methods should be private (unless we're
+	  supporting derivation).
+	- Why is Init() a separate method?  Rename it to
+	  UnixClient(AddressFamily), and call it as a
+	  constructor.
+	- Whats `active' supposed to represent?  It looks like
+	  it matches `disposed'.
+	- What's the lifetime semantics of the class?  That is,
+	  should it be possible to Connect/Close/Connect/Close
+	  multiple times?  If so, Connect should set
+	  disposed=false and shouldn't call CheckDisposed().
+	- Public parameters should follow camelCase convention
+	  (remote_end_point).
+	- Should Dispose(bool) calls client.Close() even 
+	  if s != null?
+	- I'd like it if:
+	  - Active and Client properties were removed
+	  - UnixClient had a UnixClient(Socket) constructor
+	    (thus removing the need for SetUnixClient)
+	
+UnixListener:
+	- Should implement IDisposable.  ALL types that have
+	  finalizers should implement IDisposable.
+	- Should be `sealed', unless there's a good use case for
+	  derivation in the absence of virtual methods.
+	- Public parameters should follow camelCase convention
+	  (local_end_point).
+	- Remove protected properties
+	- Consider renaming AcceptUnixClient to
+	  CreateUnixClient, and renaming AcceptSocket to Accept.
+	- Stop() never clears `server', nor does it set
+	  "active=false".  Can Stop() be called multiple times?
+	  (It could be invoked concurrently between the
+	  finalizer thread and the app thread).  Can
+	  server.Shutdown() and server.Close() be called
+	  concurrently?  Is this a security issue?
+	- File binding should behave as XSP does (try to
+	  Connect(), and if it works, fail with an
+	  InvalidOperationException.  The library shouldn't
+	  delete files unless explicitly told to.
+	- Provide a Start() overload to accept the backlog size
+	  (solving the comment in Stop());
+