Fixes leak of native File objects.

sdk/lib/io/file_impl.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of dart.io;
 
 // Read the file in blocks of size 64k.
 const int _BLOCK_SIZE = 64 * 1024;
 
 
@@ skipping 546 matching lines @@
 
   String toString() => "File: '$path'";
 
   static throwIfError(Object result, String msg, String path) {
     if (result is OSError) {
       throw new FileSystemException(msg, path, result);
     }
   }
 }
 
+abstract class _RandomAccessFileOps {
+  external factory _RandomAccessFileOps(int pointer);

[Ivan Posva, 2016/04/20 05:31:06]

Needs a patch in sdk/lib/_internal/js_runtime/lib/io_patch.dart

[zra, 2016/04/20 17:07:52]

Done.

 
-class _RandomAccessFile
-    implements RandomAccessFile {
+  int getPointer();
+  int close();
+  readByte();
+  read(int bytes);
+  readInto(List<int> buffer, int start, int end);
+  writeByte(int value);
+  writeFrom(List<int> buffer, int start, int end);
+  position();
+  setPosition(int position);
+  truncate(int length);
+  length();
+  flush();
+  lock(int lock, int start, int end);
+}
+
+class _RandomAccessFile implements RandomAccessFile {
   static bool _connectedResourceHandler = false;
 
   final String path;
-  int _id;
+
+  // Calling this function will increase the reference count on the native
+  // object that implements the file operations. It should only be called to
+  // pass the pointer to the IO Service, which will decrement the reference
+  // count when it is finished with it.
+  int _pointer() => _ops.getPointer();
+
   bool _asyncDispatched = false;
   SendPort _fileService;
 
   _FileResourceInfo _resourceInfo;
+  _RandomAccessFileOps _ops;
 
-  _RandomAccessFile(this._id, this.path) {
+  _RandomAccessFile(int pointer, this.path) {
+    _ops = new _RandomAccessFileOps(pointer);
     _resourceInfo = new _FileResourceInfo(this);
     _maybeConnectHandler();
   }
 
   void _maybePerformCleanup() {
     if (closed) {
       _FileResourceInfo.FileClosed(_resourceInfo);
     }
   }
 
-  external static int _getFD(int id);
-
   _maybeConnectHandler() {
     if (!_connectedResourceHandler) {
-      // TODO(ricow): we probably need set these in some initialization code.
-      // We need to make sure that these are always awailable from the
+      // TODO(ricow): We probably need to set these in some initialization code.
+      // We need to make sure that these are always available from the
       // observatory even if no files (or sockets for the socket ones) are
       // open.
       registerExtension('ext.dart.io.getOpenFiles',
                         _FileResourceInfo.getOpenFiles);
       registerExtension('ext.dart.io.getFileByID',
                         _FileResourceInfo.getFileInfoMapByID);
       _connectedResourceHandler = true;
     }
   }
 
   Future<RandomAccessFile> close() {
-    return _dispatch(_FILE_CLOSE, [_id], markClosed: true).then((result) {
+    return _dispatch(_FILE_CLOSE, [_pointer()], markClosed: true).then((result) {
       if (result != -1) {
-        _id = result;
+        closed = closed || (result == 0);
         _maybePerformCleanup();
         return this;
       } else {
         throw new FileSystemException("Cannot close file", path);
       }
     });
   }
 
-  external static int _close(int id);
-
   void closeSync() {
     _checkAvailable();
-    var id = _close(_id);
+    var id = _ops.close();
     if (id == -1) {
       throw new FileSystemException("Cannot close file", path);
     }
-    _id = id;
+    closed = closed || (id == 0);
     _maybePerformCleanup();
   }
 
   Future<int> readByte() {
-    return _dispatch(_FILE_READ_BYTE, [_id]).then((response) {
+    return _dispatch(_FILE_READ_BYTE, [_pointer()]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "readByte failed", path);
       }
       _resourceInfo.addRead(1);
       return response;
     });
   }
 
-  external static _readByte(int id);
-
   int readByteSync() {
     _checkAvailable();
-    var result = _readByte(_id);
+    var result = _ops.readByte();
     if (result is OSError) {
       throw new FileSystemException("readByte failed", path, result);
     }
     _resourceInfo.addRead(1);
     return result;
   }
 
   Future<List<int>> read(int bytes) {
     if (bytes is !int) {
       throw new ArgumentError(bytes);
     }
-    return _dispatch(_FILE_READ, [_id, bytes]).then((response) {
+    return _dispatch(_FILE_READ, [_pointer(), bytes]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "read failed", path);
       }
       _resourceInfo.addRead(response[1].length);
       return response[1];
     });
   }
 
-  external static _read(int id, int bytes);
-
   List<int> readSync(int bytes) {
     _checkAvailable();
     if (bytes is !int) {
       throw new ArgumentError(bytes);
     }
-    var result = _read(_id, bytes);
+    var result = _ops.read(bytes);
     if (result is OSError) {
       throw new FileSystemException("readSync failed", path, result);
     }
     _resourceInfo.addRead(result.length);
     return result;
   }
 
   Future<int> readInto(List<int> buffer, [int start = 0, int end]) {
-    if (buffer is !List ||
-        (start != null && start is !int) ||
-        (end != null && end is !int)) {
+    if ((buffer is !List) ||
+        ((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int))) {
       throw new ArgumentError();
     }
     end = RangeError.checkValidRange(start, end, buffer.length);
-    if (end == start) return new Future.value(0);
+    if (end == start) {
+      return new Future.value(0);
+    }
     int length = end - start;
-    return _dispatch(_FILE_READ_INTO, [_id, length]).then((response) {
+    return _dispatch(_FILE_READ_INTO, [_pointer(), length]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "readInto failed", path);
       }
       var read = response[1];
       var data = response[2];
       buffer.setRange(start, start + read, data);
       _resourceInfo.addRead(read);
       return read;
     });
   }
 
-  external static _readInto(int id, List<int> buffer, int start, int end);
-
   int readIntoSync(List<int> buffer, [int start = 0, int end]) {
     _checkAvailable();
-    if (buffer is !List ||
-        (start != null && start is !int) ||
-        (end != null && end is !int)) {
+    if ((buffer is !List) ||
+        ((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int))) {
       throw new ArgumentError();
     }
     end = RangeError.checkValidRange(start, end, buffer.length);
-    if (end == start) return 0;
-    var result = _readInto(_id, buffer, start, end);
+    if (end == start) {
+      return 0;
+    }
+    var result = _ops.readInto(buffer, start, end);
     if (result is OSError) {
       throw new FileSystemException("readInto failed", path, result);
     }
     _resourceInfo.addRead(result);
     return result;
   }
 
   Future<RandomAccessFile> writeByte(int value) {
     if (value is !int) {
       throw new ArgumentError(value);
     }
-    return _dispatch(_FILE_WRITE_BYTE, [_id, value]).then((response) {
+    return _dispatch(_FILE_WRITE_BYTE, [_pointer(), value]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "writeByte failed", path);
       }
       _resourceInfo.addWrite(1);
       return this;
     });
   }
 
-  external static _writeByte(int id, int value);
-
   int writeByteSync(int value) {
     _checkAvailable();
     if (value is !int) {
       throw new ArgumentError(value);
     }
-    var result = _writeByte(_id, value);
+    var result = _ops.writeByte(value);
     if (result is OSError) {
       throw new FileSystemException("writeByte failed", path, result);
     }
     _resourceInfo.addWrite(1);
     return result;
   }
 
   Future<RandomAccessFile> writeFrom(
       List<int> buffer, [int start = 0, int end]) {
     if ((buffer is !List) ||
-        (start != null && start is !int) ||
-        (end != null && end is !int)) {
+        ((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int))) {
       throw new ArgumentError("Invalid arguments to writeFrom");
     }
     end = RangeError.checkValidRange(start, end, buffer.length);
-    if (end == start) return new Future.value(this);
+    if (end == start) {
+      return new Future.value(this);
+    }
     _BufferAndStart result;
     try {
       result = _ensureFastAndSerializableByteData(buffer, start, end);
     } catch (e) {
       return new Future.error(e);
     }
 
     List request = new List(4);
-    request[0] = _id;
+    request[0] = _pointer();
     request[1] = result.buffer;
     request[2] = result.start;
     request[3] = end - (start - result.start);
     return _dispatch(_FILE_WRITE_FROM, request).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "writeFrom failed", path);
       }
       _resourceInfo.addWrite(end - (start - result.start));
       return this;
     });
   }
 
-  external static _writeFrom(int id, List<int> buffer, int start, int end);
-
   void writeFromSync(List<int> buffer, [int start = 0, int end]) {
     _checkAvailable();
-    if (buffer is !List ||
-        (start != null && start is !int) ||
-        (end != null && end is !int)) {
+    if ((buffer is !List) ||
+        ((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int))) {
       throw new ArgumentError("Invalid arguments to writeFromSync");
     }
     end = RangeError.checkValidRange(start, end, buffer.length);
-    if (end == start) return;
+    if (end == start) {
+      return;
+    }
     _BufferAndStart bufferAndStart =
         _ensureFastAndSerializableByteData(buffer, start, end);
-    var result = _writeFrom(_id,
-                            bufferAndStart.buffer,
-                            bufferAndStart.start,
-                            end - (start - bufferAndStart.start));
+    var result = _ops.writeFrom(bufferAndStart.buffer,
+                                  bufferAndStart.start,
+                                  end - (start - bufferAndStart.start));
     if (result is OSError) {
       throw new FileSystemException("writeFrom failed", path, result);
     }
     _resourceInfo.addWrite(end - (start - bufferAndStart.start));
   }
 
   Future<RandomAccessFile> writeString(String string,
                                        {Encoding encoding: UTF8}) {
     if (encoding is! Encoding) {
       throw new ArgumentError(encoding);
     }
     var data = encoding.encode(string);
     return writeFrom(data, 0, data.length);
   }
 
   void writeStringSync(String string, {Encoding encoding: UTF8}) {
     if (encoding is! Encoding) {
       throw new ArgumentError(encoding);
     }
     var data = encoding.encode(string);
     writeFromSync(data, 0, data.length);
   }
 
   Future<int> position() {
-    return _dispatch(_FILE_POSITION, [_id]).then((response) {
+    return _dispatch(_FILE_POSITION, [_pointer()]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "position failed", path);
       }
       return response;
     });
   }
 
-  external static _position(int id);
-
   int positionSync() {
     _checkAvailable();
-    var result = _position(_id);
+    var result = _ops.position();
     if (result is OSError) {
       throw new FileSystemException("position failed", path, result);
     }
     return result;
   }
 
   Future<RandomAccessFile> setPosition(int position) {
-    return _dispatch(_FILE_SET_POSITION, [_id, position])
+    return _dispatch(_FILE_SET_POSITION, [_pointer(), position])
         .then((response) {
           if (_isErrorResponse(response)) {
             throw _exceptionFromResponse(response, "setPosition failed", path);
           }
           return this;
         });
   }
 
-  external static _setPosition(int id, int position);
-
   void setPositionSync(int position) {
     _checkAvailable();
-    var result = _setPosition(_id, position);
+    var result = _ops.setPosition(position);
     if (result is OSError) {
       throw new FileSystemException("setPosition failed", path, result);
     }
   }
 
   Future<RandomAccessFile> truncate(int length) {
-    return _dispatch(_FILE_TRUNCATE, [_id, length]).then((response) {
+    return _dispatch(_FILE_TRUNCATE, [_pointer(), length]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "truncate failed", path);
       }
       return this;
     });
   }
 
-  external static _truncate(int id, int length);
-
   void truncateSync(int length) {
     _checkAvailable();
-    var result = _truncate(_id, length);
+    var result = _ops.truncate(length);
     if (result is OSError) {
       throw new FileSystemException("truncate failed", path, result);
     }
   }
 
   Future<int> length() {
-    return _dispatch(_FILE_LENGTH, [_id]).then((response) {
+    return _dispatch(_FILE_LENGTH, [_pointer()]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response, "length failed", path);
       }
       return response;
     });
   }
 
-  external static _length(int id);
-
   int lengthSync() {
     _checkAvailable();
-    var result = _length(_id);
+    var result = _ops.length();
     if (result is OSError) {
       throw new FileSystemException("length failed", path, result);
     }
     return result;
   }
 
   Future<RandomAccessFile> flush() {
-    return _dispatch(_FILE_FLUSH, [_id]).then((response) {
+    return _dispatch(_FILE_FLUSH, [_pointer()]).then((response) {
       if (_isErrorResponse(response)) {
         throw _exceptionFromResponse(response,
                                      "flush failed",
                                      path);
       }
       return this;
     });
   }
 
-  external static _flush(int id);
-
   void flushSync() {
     _checkAvailable();
-    var result = _flush(_id);
+    var result = _ops.flush();
     if (result is OSError) {
       throw new FileSystemException("flush failed", path, result);
     }
   }
 
   static final int LOCK_UNLOCK = 0;
   static final int LOCK_SHARED = 1;
   static final int LOCK_EXCLUSIVE = 2;
 
   Future<RandomAccessFile> lock(
       [FileLock mode = FileLock.EXCLUSIVE, int start = 0, int end]) {
-    if ((start != null && start is !int) ||
-        (end != null && end is !int) ||
-         mode is !FileLock) {
+    if (((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int)) ||
+         (mode is !FileLock)) {
       throw new ArgumentError();
     }
-    if (start == null) start = 0;
-    if (end == null) end = -1;
-    if (start < 0 || end < -1 || (end != -1 && start >= end)) {
+    if (start == null) {
+      start = 0;
+    }
+    if (end == null) {
+      end = -1;
+    }
+    if ((start < 0) || (end < -1) || ((end != -1) && (start >= end))) {
       throw new ArgumentError();
     }
-    int lock = mode == FileLock.EXCLUSIVE ? LOCK_EXCLUSIVE : LOCK_SHARED;
-    return _dispatch(_FILE_LOCK, [_id, lock, start, end])
+    int lock = (mode == FileLock.EXCLUSIVE) ? LOCK_EXCLUSIVE : LOCK_SHARED;
+    return _dispatch(_FILE_LOCK, [_pointer(), lock, start, end])
         .then((response) {
           if (_isErrorResponse(response)) {
             throw _exceptionFromResponse(response, 'lock failed', path);
           }
           return this;
         });
   }
 
   Future<RandomAccessFile> unlock([int start = 0, int end]) {
-    if ((start != null && start is !int) ||
-        (end != null && end is !int)) {
+    if (((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int))) {
       throw new ArgumentError();
     }
-    if (start == null) start = 0;
-    if (end == null) end = -1;
-    if (start == end) throw new ArgumentError();
-    return _dispatch(_FILE_LOCK, [_id, LOCK_UNLOCK, start, end])
+    if (start == null) {
+      start = 0;
+    }
+    if (end == null) {
+      end = -1;
+    }
+    if (start == end) {
+      throw new ArgumentError();
+    }
+    return _dispatch(_FILE_LOCK, [_pointer(), LOCK_UNLOCK, start, end])
         .then((response) {
           if (_isErrorResponse(response)) {
             throw _exceptionFromResponse(response, 'unlock failed', path);
           }
           return this;
         });
   }
 
-  external static _lock(int id, int lock, int start, int end);
-
   void lockSync([FileLock mode = FileLock.EXCLUSIVE, int start = 0, int end]) {
     _checkAvailable();
-    if ((start != null && start is !int) ||
-        (end != null && end is !int) ||
-         mode is !FileLock) {
+    if (((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int)) ||
+         (mode is !FileLock)) {
       throw new ArgumentError();
     }
-    if (start == null) start = 0;
-    if (end == null) end = -1;
-    if (start < 0 || end < -1 || (end != -1 && start >= end)) {
+    if (start == null) {

[Ivan Posva, 2016/04/20 05:31:06]

This test is a bit confusing to say the least. It looks as if it is a left over
from a non-default value being set for start.

Similarly the test above is a bit strange that it is checking for null as if
null was a legal parameter value. Which I believe it is not...

Same applies to end which should probably get a -1 as default value, no?

[zra, 2016/04/20 17:07:52]

Done.

+      start = 0;
+    }
+    if (end == null) {
+      end = -1;
+    }
+    if ((start < 0) || (end < -1) || ((end != -1) && (start >= end))) {
       throw new ArgumentError();
     }
-    int lock = mode == FileLock.EXCLUSIVE ? LOCK_EXCLUSIVE : LOCK_SHARED;
-    var result = _lock(_id, lock, start, end);
+    int lock = (mode == FileLock.EXCLUSIVE) ? LOCK_EXCLUSIVE : LOCK_SHARED;
+    var result = _ops.lock(lock, start, end);
     if (result is OSError) {
       throw new FileSystemException('lock failed', path, result);
     }
   }
 
   void unlockSync([int start = 0, int end]) {
     _checkAvailable();
-    if ((start != null && start is !int) ||
-        (end != null && end is !int)) {
+    if (((start != null) && (start is !int)) ||
+        ((end != null) && (end is !int))) {
       throw new ArgumentError();
     }
-    if (start == null) start = 0;
-    if (end == null) end = -1;
-    if (start == end) throw new ArgumentError();
-    var result = _lock(_id, LOCK_UNLOCK, start, end);
+    if (start == null) {

[Ivan Posva, 2016/04/20 05:31:06]

ditto

[zra, 2016/04/20 17:07:52]

Done.

+      start = 0;
+    }
+    if (end == null) {
+      end = -1;
+    }
+    if (start == end) {
+      throw new ArgumentError();
+    }
+    var result = _ops.lock(LOCK_UNLOCK, start, end);
     if (result is OSError) {
       throw new FileSystemException('unlock failed', path, result);
     }
   }
 
-  bool get closed => _id == 0;
+  bool closed = false;
 
   Future _dispatch(int request, List data, { bool markClosed: false }) {
     if (closed) {
       return new Future.error(new FileSystemException("File closed", path));
     }
     if (_asyncDispatched) {
       var msg = "An async operation is currently pending";
       return new Future.error(new FileSystemException(msg, path));
     }
     if (markClosed) {
-      // Set the id_ to 0 (NULL) to ensure the no more async requests
-      // can be issued for this file.
-      _id = 0;
+      // Set closed to true to ensure that no more async requests can be issued
+      // for this file.
+      closed = true;
     }
     _asyncDispatched = true;
     return _IOService._dispatch(request, data)
         .whenComplete(() {
           _asyncDispatched = false;
         });
   }
 
   void _checkAvailable() {
     if (_asyncDispatched) {
       throw new FileSystemException("An async operation is currently pending",
                                     path);
     }
     if (closed) {
       throw new FileSystemException("File closed", path);
     }
   }
 }