Extract meta data about open sockets into separate class.

runtime/bin/socket_patch.dart

Index: runtime/bin/socket_patch.dart
diff --git a/runtime/bin/socket_patch.dart b/runtime/bin/socket_patch.dart
index 94285aaaa02ed59b6eb0905770a60a066762bdf3..1947d74b6f17c64298c6921dbd2d989ff86c333d 100644
--- a/runtime/bin/socket_patch.dart
+++ b/runtime/bin/socket_patch.dart
@@ -280,9 +280,6 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
   static const Duration _RETRY_DURATION_LOOPBACK =
       const Duration(milliseconds: 25);
 
-  // Use default Map so we keep order.
-  static Map<int, _NativeSocket> _sockets = new Map<int, _NativeSocket>();
-
   // Socket close state
   bool isClosed = false;
   bool isClosing = false;
@@ -317,13 +314,9 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
   bool writeAvailable = false;
 
   static final Stopwatch sw = new Stopwatch()..start();
-  // Statistics.
-  int totalRead = 0;
-  int totalWritten = 0;
-  int readCount = 0;
-  int writeCount = 0;
-  double lastRead;
-  double lastWrite;
+
+  static bool connectedResourceHandler = false;
+  _ReadWriteResourceInfo resourceInfo;
 
   // The owner object is the object that the Socket is being used by, e.g.
   // a HttpServer, a WebSocket connection, a process pipe, etc.
@@ -441,6 +434,8 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
                   _RETRY_DURATION_LOOPBACK :
                   _RETRY_DURATION;
               var timer = new Timer(duration, connectNext);
+              setupResourceInfo(socket);

[Søren Gjesse, 2015/09/02 09:07:49]

Shouldn't this move down to just after timer.cancel() below?

What is happening with connectNext is that if a destination have more than one
IP address as the result of a DNS lookup each of these are tired with a timeout,
and the first that actually reports as being connected will close any other open
ones in the "connecting" Map.

[ricow1, 2015/09/02 10:45:42]

If I do that, and there is an error, I do a socket.close(), which will trigger
the resource info to be removed (but we can't remove it, because it is not
there)

+
               connecting[socket] = timer;
               // Setup handlers for receiving the first write event which
               // indicate that the socket is fully connected.
@@ -492,7 +487,6 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
         .then((address) {
           var socket = new _NativeSocket.listen();
           socket.localAddress = address;
-
           var result = socket.nativeCreateBindListen(address._in_addr,
                                                      port,
                                                      backlog,
@@ -505,11 +499,16 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
                                       port: port);
           }
           if (port != 0) socket.localPort = port;
+          setupResourceInfo(socket);
           socket.connectToEventHandler();
           return socket;
         });
   }
 
+  static void setupResourceInfo(_NativeSocket socket) {
+    socket.resourceInfo = new _SocketResourceInfo(socket);
+  }
+
   static Future<_NativeSocket> bindDatagram(
       host, int port, bool reuseAddress) {
     return new Future.value(host)
@@ -534,6 +533,7 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
                                       port: port);
           }
           if (port != 0) socket.localPort = port;
+          setupResourceInfo(socket);
           return socket;
         });
   }
@@ -575,10 +575,14 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
     }
     if (result != null) {
       available -= result.length;
-      totalRead += result.length;
+      if (!isInternal && !isPipe) {

[ricow1, 2015/09/01 12:06:24]

These "if (!isInternal && !isPipe) {" will go away when I add support for pipes
and internal uses. The fields used is already in the superclass

[Søren Gjesse, 2015/09/02 09:07:49]

Please add a TODO then.

Maybe check if resourceInfo is non-null instead (in the other places as well).

[ricow1, 2015/09/02 10:45:42]

So the reason I did not want to do that is that I want to actually catch cases
where we did not setup resouceInfo, but where not internal or pipe (i.e., we
could get into a situation where we had an open socket without a resourceInfo
object, which we then would not see in the observatory). I can change this to:
if (resourceInfo != null) {
  resourceInfo.readCount++;
  resourceInfo.lastRead = timestamp;
} else {
  assert(isInternal or isPipe);
}
but that is more verbose. I have no strong feelings towards one or the other,
but would like to at least keep the coverage, I will let you decide. Added TODO
here and the other places.

[Søren Gjesse, 2015/09/02 12:24:49]

I like the more verbose version with the assert.

[ricow1, 2015/09/02 12:45:50]

Done (instead I put the assert before)

+        resourceInfo.totalRead += result.length;
+      }
+    }
+    if (!isInternal && !isPipe) {
+      resourceInfo.readCount++;
+      resourceInfo.lastRead = timestamp;
     }
-    readCount++;
-    lastRead = timestamp;
     return result;
   }
 
@@ -595,10 +599,14 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
       // receive. If available becomes > 0, the _NativeSocket will continue to
       // emit read events.
       available = nativeAvailable();
-      totalRead += result.data.length;
+      if (!isInternal && !isPipe) {
+        resourceInfo.totalRead += result.data.length;
+      }
+    }
+    if (!isInternal && !isPipe) {
+      resourceInfo.readCount++;
+      resourceInfo.lastRead = timestamp;
     }
-    readCount++;
-    lastRead = timestamp;
     return result;
   }
 
@@ -637,9 +645,11 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
     }
     // Negate the result, as stated above.
     if (result < 0) result = -result;
-    totalWritten += result;
-    writeCount++;
-    lastWrite = timestamp;
+    if (!isInternal && !isPipe) {
+      resourceInfo.totalWritten += result;
+      resourceInfo.writeCount++;
+      resourceInfo.lastWrite = timestamp;
+    }
     return result;
   }
 
@@ -656,9 +666,11 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
       scheduleMicrotask(() => reportError(result, "Send failed"));
       result = 0;
     }
-    totalWritten += result;
-    writeCount++;
-    lastWrite = timestamp;
+    if (!isInternal && !isPipe) {
+      resourceInfo.totalWritten += result;
+      resourceInfo.writeCount++;
+      resourceInfo.lastWrite = timestamp;
+    }
     return result;
   }
 
@@ -673,8 +685,11 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
     if (nativeAccept(socket) != true) return null;
     socket.localPort = localPort;
     socket.localAddress = address;
-    totalRead += 1;
-    lastRead = timestamp;
+    setupResourceInfo(socket);
+    if (!isInternal && !isPipe) {
+      resourceInfo.totalRead += 1;
+      resourceInfo.lastRead = timestamp;
+    }
     return socket;
   }
 
@@ -787,6 +802,9 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
         if (i == DESTROYED_EVENT) {
           assert(isClosing);
           assert(!isClosed);
+          if (!isPipe && !isInternal) {
+            _SocketResourceInfo.SocketClosed(resourceInfo);
+          }
           isClosed = true;
           closeCompleter.complete();
           disconnectFromEventHandler();
@@ -904,7 +922,14 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
     assert(!isClosed);
     if (eventPort == null) {
       eventPort = new RawReceivePort(multiplex);
-      _sockets[_serviceId] = this;
+    }
+    if (!connectedResourceHandler) {

[ricow1, 2015/09/01 12:06:25]

I am not sure this is the correct location for this, if you have a better place
let me know.

[Søren Gjesse, 2015/09/02 12:24:49]

I think this is fine for now.

+      registerExtension('__getOpenSockets',
+                        _SocketResourceInfo.getOpenSockets);
+      registerExtension('__getSocketByID',
+                        _SocketResourceInfo.getSocketInfoMapByID);
+
+      connectedResourceHandler = true;
     }
   }
 
@@ -912,7 +937,6 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
     assert(eventPort != null);
     eventPort.close();
     eventPort = null;
-    _sockets.remove(_serviceId);
     // Now that we don't track this Socket anymore, we can clear the owner
     // field.
     owner = null;
@@ -1017,123 +1041,6 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
     if (result is OSError) throw result;
   }
 
-  String get _serviceTypePath => 'io/sockets';
-  String get _serviceTypeName => 'Socket';
-
-  String _JSONKind() {
-    return isListening ? "Listening" :
-           isPipe ? "Pipe" :
-           isInternal ? "Internal" : "Normal";
-  }
-
-  Map _toJSONPipe(bool ref) {
-    var name = 'Anonymous Pipe';
-    var r = {
-      'id': _servicePath,
-      'type': _serviceType(ref),
-      'name': name,
-      'user_name': name,
-      'kind': _JSONKind(),
-    };
-    if (ref) {
-      return r;
-    }
-    r['readClosed'] = isClosedRead;
-    r['writeClosed'] = isClosedWrite;
-    r['closing'] = isClosing;
-    r['fd'] = nativeGetSocketId();
-    if (owner != null) {
-      r['owner'] = owner._toJSON(true);
-    }
-    return r;
-  }
-
-  Map _toJSONInternal(bool ref) {
-    var name = 'Internal';
-    var r = {
-      'id': _servicePath,
-      'type': _serviceType(ref),
-      'name': name,
-      'user_name': name,
-      'kind': _JSONKind(),
-    };
-    if (ref) {
-      return r;
-    }
-    r['closing'] = isClosing;
-    r['fd'] = nativeGetSocketId();
-    if (owner != null) {
-      r['owner'] = owner._toJSON(true);
-    }
-    return r;
-  }
-
-  Map _toJSONNetwork(bool ref) {
-    var name = '${address.host}:$port';
-    if (isTcp && !isListening) name += " <-> ${remoteAddress.host}:$remotePort";
-    var r = {
-      'id': _servicePath,
-      'type': _serviceType(ref),
-      'name': name,
-      'user_name': name,
-      'kind': _JSONKind(),
-    };
-    if (ref) {
-      return r;
-    }
-    var protocol = isTcp ? "TCP" : isUdp ? "UDP" : null;
-    var localAddress;
-    var localPort;
-    var rAddress;
-    var rPort;
-    try {
-      localAddress = address.address;
-    } catch (e) { }
-    try {
-      localPort = port;
-    } catch (e) { }
-    try {
-      rAddress = this.remoteAddress.address;
-    } catch (e) { }
-    try {
-      rPort = remotePort;
-    } catch (e) { }
-    r['localAddress'] = localAddress;
-    r['localPort'] = localPort;
-    r['remoteAddress'] = rAddress;
-    r['remotePort'] = rPort;
-    r['protocol'] = protocol;
-    r['readClosed'] = isClosedRead;
-    r['writeClosed'] = isClosedWrite;
-    r['closing'] = isClosing;
-    r['listening'] = isListening;
-    r['fd'] = nativeGetSocketId();
-    if (owner != null) {
-      r['owner'] = owner._toJSON(true);
-    }
-    return r;
-  }
-
-  Map _toJSON(bool ref) {
-    var map;
-    if (isPipe) {
-      map =  _toJSONPipe(ref);
-    } else if (isInternal) {
-      map = _toJSONInternal(ref);
-    } else {
-      map = _toJSONNetwork(ref);
-    }
-    if (!ref) {
-      map['available'] = available;
-      map['totalRead'] = totalRead;
-      map['totalWritten'] = totalWritten;
-      map['readCount'] = totalWritten;
-      map['writeCount'] = writeCount;
-      map['lastRead'] = lastRead;
-      map['lastWrite'] = lastWrite;
-    }
-    return map;
-  }
 
   void nativeSetSocketId(int id) native "Socket_SetSocketId";
   nativeAvailable() native "Socket_Available";
@@ -1286,8 +1193,6 @@ class _RawServerSocket extends Stream<RawSocket>
     return new _RawServerSocketReference(_referencePort.sendPort);
   }
 
-  Map _toJSON(bool ref) => _socket._toJSON(ref);
-
   void set _owner(owner) { _socket.owner = owner; }
 }
 
@@ -1465,7 +1370,6 @@ class _RawSocket extends Stream<RawSocketEvent>
     }
   }
 
-  Map _toJSON(bool ref) => _socket._toJSON(ref);
   void set _owner(owner) { _socket.owner = owner; }
 }
 
@@ -1528,8 +1432,6 @@ class _ServerSocket extends Stream<Socket>
     return new _ServerSocketReference(_socket.reference);
   }
 
-  Map _toJSON(bool ref) => _socket._toJSON(ref);
-
   void set _owner(owner) { _socket._owner = owner; }
 }
 
@@ -1843,7 +1745,6 @@ class _Socket extends Stream<List<int>> implements Socket {
     }
   }
 
-  Map _toJSON(bool ref) => _raw._toJSON(ref);
   void set _owner(owner) { _raw._owner = owner; }
 }
 
@@ -1999,4 +1900,3 @@ Datagram _makeDatagram(List<int> data,
       port);
 }
 
-String _socketsStats() => _SocketsObservatory.toJSON();