Resolution for issue #5092: Unit test handle checks consider dangling handles to be valid.

runtime/vm/unit_test.h

Index: runtime/vm/unit_test.h
diff --git a/runtime/vm/unit_test.h b/runtime/vm/unit_test.h
index c08f36efae6178098554acd010ee419041b60d5f..59f742f57905e981a4ef969b8af6f1b6570361d8 100644
--- a/runtime/vm/unit_test.h
+++ b/runtime/vm/unit_test.h
@@ -547,6 +547,13 @@ class CompilerTest : public AllStatic {
               "    '%s'\n",                                                    \
               #handle, Dart_GetError(tmp_handle));                             \
     }                                                                          \
+    if (!Dart_IsValid(tmp_handle)) {                                           \

[bkonyi, 2017/01/18 00:47:01]

I'm not sure if we'd rather create a new macro or just modify EXPECT_VALID to
check for dangling handles. Personally, this makes the most sense to me as it
won't require any changes to the current tests.

[siva, 2017/01/18 23:27:06]

The check for the handle being valid should happen before the check for
Dart_IsError.

[bkonyi, 2017/01/19 01:05:13]

Done.

+      dart::Expect(__FILE__, __LINE__)                                         \
+          .Fail(                                                               \
+              "expected '%s' to be a valid handle but '%s' has already been "  \
+              "freed\n",                                                       \
+              #handle, #handle);                                               \
+    }                                                                          \
   } while (0)
 
 #define EXPECT_ERROR(handle, substring)                                        \
@@ -564,6 +571,18 @@ class CompilerTest : public AllStatic {
     }                                                                          \
   } while (0)
 
+#define EXPECT_INVALID(handle)                                                 \
+  do {                                                                         \

[bkonyi, 2017/01/18 00:44:47]

Should this check for error handles as well, should it be part of EXPECT_ERROR,
or is this fine the way it is?

[siva, 2017/01/18 23:27:06]

Why do we need this macro, couldn't the unit tests to
EXPECT(!Dart_IsValid(handle());

[bkonyi, 2017/01/19 01:05:13]

You're right. Removed.

+    Dart_Handle tmp_handle = (handle);                                         \
+    if (Dart_IsValid(tmp_handle)) {                                            \
+      dart::Expect(__FILE__, __LINE__)                                         \
+          .Fail(                                                               \
+              "expected '%s' to be an invalid handle but '%s' has not been "   \
+              "freed\n",                                                       \
+              #handle, #handle);                                               \
+    }                                                                          \
+  } while (0)
+
 #define EXPECT_TRUE(handle)                                                    \
   do {                                                                         \
     Dart_Handle tmp_handle = (handle);                                         \