commit b7959afdc4fa4acb65c67cafa6985394633138c8
author Zachary Anderson <zra@google.com> Thu Mar 12 09:18:21 2015 -0700
committer Zachary Anderson <zra@google.com> Thu Mar 12 09:18:21 2015 -0700
tree b299f04fa872d554eeb64e3d1e8f890738db5271
parent 71f560743247332eb69bb05e64ba63965ded99df

Dart: Better handle leak checks. close() is async.

This change adds tracking of MojoHandle creation and closing in Debug
mode, and makes it possible to emit a log of leaked handles on Mojo app
shutdown. If any handles are leaked, causing the finalizer to need to
close a handle, an error message is emitted with LOG(ERROR). All of the
handles created by an isolate are also closed on an unhandled exception.
This should prevent hangs.

To aid tracking handles, close() now returns a Future in most cases.
This is because a call to close() will generally send a command to the
handle watcher, and so the actual close() of the underlying handle
happens asynchronously. In particular when the future for a call to
close() on an Application resolves, we should be able to do:

assert(MojoHandle.reportLeakedHandles());

This change also fixes analyzer warnings and removes the special case
to ignore warnings from the analyzer's wrapper script.

BUG=
R=erg@chromium.org, sky@chromium.org

Review URL: https://codereview.chromium.org/996923003

mojo/dart/embedder/mojo_natives.cc

diff --git a/mojo/dart/embedder/mojo_natives.cc b/mojo/dart/embedder/mojo_natives.cc
index cc41767..170a3dd 100644
--- a/mojo/dart/embedder/mojo_natives.cc
+++ b/mojo/dart/embedder/mojo_natives.cc
@@ -4,6 +4,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <set>
 #include <vector>
 
 #include "base/logging.h"
@@ -11,6 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "dart/runtime/include/dart_api.h"
 #include "mojo/dart/embedder/builtin.h"
+#include "mojo/dart/embedder/isolate_data.h"
 #include "mojo/public/c/system/core.h"
 #include "mojo/public/cpp/system/core.h"
 
@@ -122,7 +124,13 @@
   CloserCallbackPeer* callback_peer =
       reinterpret_cast<CloserCallbackPeer*>(peer);
   if (callback_peer->handle != MOJO_HANDLE_INVALID) {
-    MojoClose(callback_peer->handle);
+    MojoResult res = MojoClose(callback_peer->handle);
+    if (res == MOJO_RESULT_OK) {
+      // If this finalizer callback successfully closes a handle, it means that
+      // the handle has leaked from the Dart code, which is an error.
+      LOG(ERROR) << "Handle Finalizer closing handle:\n\tisolate: "
+                 << "\n\thandle: " << callback_peer->handle;
+    }
   }
   delete callback_peer;
 }
@@ -167,8 +175,16 @@
     return;
   }
 
+  // Add the handle to this isolate's set.
+  MojoHandle handle = static_cast<MojoHandle>(raw_handle);
+  Dart_Isolate isolate = Dart_CurrentIsolate();
+  void* data = Dart_IsolateData(isolate);
+  IsolateData* isolate_data = reinterpret_cast<IsolateData*>(data);
+  isolate_data->unclosed_handles.insert(handle);
+
+  // Set up a finalizer.
   CloserCallbackPeer* callback_peer = new CloserCallbackPeer();
-  callback_peer->handle = static_cast<MojoHandle>(raw_handle);
+  callback_peer->handle = handle;
   Dart_NewWeakPersistentHandle(mojo_handle_instance,
                                reinterpret_cast<void*>(callback_peer),
                                sizeof(CloserCallbackPeer),
@@ -177,10 +193,17 @@
 }
 
 void MojoHandle_Close(Dart_NativeArguments arguments) {
-  int64_t handle;
-  CHECK_INTEGER_ARGUMENT(arguments, 0, &handle, InvalidArgument);
+  int64_t raw_handle;
+  CHECK_INTEGER_ARGUMENT(arguments, 0, &raw_handle, InvalidArgument);
 
-  MojoResult res = MojoClose(static_cast<MojoHandle>(handle));
+  // Remove the handle from this isolate's set.
+  MojoHandle handle = static_cast<MojoHandle>(raw_handle);
+  Dart_Isolate isolate = Dart_CurrentIsolate();
+  void* data = Dart_IsolateData(isolate);
+  IsolateData* isolate_data = reinterpret_cast<IsolateData*>(data);
+  isolate_data->unclosed_handles.erase(handle);
+
+  MojoResult res = MojoClose(handle);
 
   Dart_SetIntegerReturnValue(arguments, static_cast<int64_t>(res));
 }