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

mojo/dart/embedder/mojo_natives.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stdio.h>
 #include <string.h>
+#include <set>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/macros.h"
 #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"
 
 namespace mojo {
 namespace dart {
 
 #define MOJO_NATIVE_LIST(V)                \
   V(MojoSharedBuffer_Create, 2)            \
   V(MojoSharedBuffer_Duplicate, 2)         \
   V(MojoSharedBuffer_Map, 5)               \
@@ skipping 91 matching lines @@
 struct CloserCallbackPeer {
   MojoHandle handle;
 };
 
 static void MojoHandleCloserCallback(void* isolate_data,
                                      Dart_WeakPersistentHandle handle,
                                      void* peer) {
   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) {
+      LOG(ERROR) << "Handle Finalizer closing handle:\n\tisolate: "

[sky, 2015/03/11 17:17:57]

Why the LOG(ERROR) here? Doesn't MOJO_RESULT_OK mean the close was successful?

[zra, 2015/03/11 18:58:20]

If the finalizer callback successfully closes a handle, it means that the handle
was leaked from the Dart code. I'll add a comment.

+                 << "\n\thandle: " << callback_peer->handle;
+    }
   }
   delete callback_peer;
 }
 
 // Setup a weak persistent handle for a Dart MojoHandle that calls MojoClose
 // on the handle when the MojoHandle is GC'd or the VM is going down.
 void MojoHandle_Register(Dart_NativeArguments arguments) {
   // An instance of Dart class MojoHandle.
   Dart_Handle mojo_handle_instance = Dart_GetNativeArgument(arguments, 0);
   if (!Dart_IsInstance(mojo_handle_instance)) {
@@ skipping 24 matching lines @@
   if (Dart_IsError(result)) {
     SetInvalidArgumentReturn(arguments);
     return;
   }
 
   if (raw_handle == static_cast<int64_t>(MOJO_HANDLE_INVALID)) {
     SetInvalidArgumentReturn(arguments);
     return;
   }
 
+  // Add the handle to this isolate's set.
+  Dart_Isolate isolate = Dart_CurrentIsolate();
+  void* data = Dart_IsolateData(isolate);
+  IsolateData* isolate_data = reinterpret_cast<IsolateData*>(data);
+  isolate_data->unclosed_handles.insert(raw_handle);
+
+  // Set up a finalizer.
   CloserCallbackPeer* callback_peer = new CloserCallbackPeer();
   callback_peer->handle = static_cast<MojoHandle>(raw_handle);
   Dart_NewWeakPersistentHandle(mojo_handle_instance,
                                reinterpret_cast<void*>(callback_peer),
                                sizeof(CloserCallbackPeer),
                                MojoHandleCloserCallback);
   Dart_SetIntegerReturnValue(arguments, static_cast<int64_t>(MOJO_RESULT_OK));
 }
 
 void MojoHandle_Close(Dart_NativeArguments arguments) {
   int64_t handle;
   CHECK_INTEGER_ARGUMENT(arguments, 0, &handle, InvalidArgument);
 
+  // Remove the handle from this isolate's set.
+  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(static_cast<MojoHandle>(handle));
 
   Dart_SetIntegerReturnValue(arguments, static_cast<int64_t>(res));
 }
 
 void MojoHandle_Wait(Dart_NativeArguments arguments) {
   int64_t handle = 0;
   int64_t signals = 0;
   int64_t deadline = 0;
   CHECK_INTEGER_ARGUMENT(arguments, 0, &handle, InvalidArgument);
@@ skipping 600 matching lines @@
   mojo_control_handle = control_handle;
   Dart_SetIntegerReturnValue(arguments, static_cast<int64_t>(MOJO_RESULT_OK));
 }
 
 void MojoHandleWatcher_GetControlHandle(Dart_NativeArguments arguments) {
   Dart_SetIntegerReturnValue(arguments, mojo_control_handle);
 }
 
 }  // namespace dart
 }  // namespace mojo