Don't pop invalid test environment

Summary: If `unset($env)` throws then we pop some other environment instead which is impossible to pop later.

Test Plan:
  $ arc unit src/infrastructure/env/__tests__ src/applications/calendar/storage/__tests__

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4488

src/infrastructure/env/PhabricatorEnv.php

@@ -293,6 +293,7 @@ final class PhabricatorEnv {
     $source = self::$sourceStack->popSource();
     $stack_key = spl_object_hash($source);
     if ($stack_key !== $key) {
+      self::$sourceStack->pushSource($source);
       throw new Exception(
         "Scoped environments were destroyed in a diffent order than they ".
         "were initialized.");

src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php

@@ -130,15 +130,11 @@ final class PhabricatorEnvTestCase extends PhabricatorTestCase {
 
   public function testOverrideOrder() {
     $outer = PhabricatorEnv::beginScopedEnv();
-    $middle = PhabricatorEnv::beginScopedEnv();
     $inner = PhabricatorEnv::beginScopedEnv();
 
     $caught = null;
     try {
-      if (phutil_is_hiphop_runtime()) {
+      $outer->__destruct();
-        $middle->__destruct();
-      }
-      unset($middle);
     } catch (Exception $ex) {
       $caught = $ex;
     }
@@ -149,25 +145,11 @@ final class PhabricatorEnvTestCase extends PhabricatorTestCase {
       "Destroying a scoped environment which is not on the top of the stack ".
       "should throw.");
 
-    $caught = null;
+    if (phutil_is_hiphop_runtime()) {
-    try {
+      $inner->__destruct();
-      if (phutil_is_hiphop_runtime()) {
-        $inner->__destruct();
-      }
-      unset($inner);
-    } catch (Exception $ex) {
-      $caught = $ex;
     }
+    unset($inner);
 
-    $this->assertEqual(
-      true,
-      $caught instanceof Exception,
-      "Destroying a scoped environment which is not on the top of the stack ".
-      "should throw.");
-
-    // Although we popped the other two out-of-order, we still expect to end
-    // up in the right state after handling the exceptions, so this should
-    // execute without issues.
     if (phutil_is_hiphop_runtime()) {
       $outer->__destruct();
     }

src/infrastructure/testing/PhabricatorTestCase.php

@@ -91,6 +91,9 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase {
     }
 
     try {
+      if (phutil_is_hiphop_runtime()) {
+        $this->env->__destruct();
+      }
       unset($this->env);
     } catch (Exception $ex) {
       throw new Exception(