Bug 1446466 Crash if moz_dispose_arena is called, and comment out all callers r?glandium draft
authorTom Ritter <tom@mozilla.com>
Wed, 21 Mar 2018 20:49:35 -0500
changeset 773273 0d3c70995ea3f547ee12c80066f11ea6f9854007
parent 773272 1161aab04222b893be4589711c38116a37950a71
child 773274 1695039d5ff8d49ef93c8aaa2e6e6637b0e524f6
push id104197
push userbmo:tom@mozilla.com
push dateTue, 27 Mar 2018 19:17:59 +0000
reviewersglandium
bugs1446466, 1364359
milestone61.0a1
Bug 1446466 Crash if moz_dispose_arena is called, and comment out all callers r?glandium Bug 1364359 is to fix a leaked arena. Until that is fixed; it is unsafe to call moz_dispose_arena more than once. MozReview-Commit-ID: KIby1RLtrPK
js/src/jsutil.cpp
memory/build/mozjemalloc.cpp
memory/gtest/TestJemalloc.cpp
--- a/js/src/jsutil.cpp
+++ b/js/src/jsutil.cpp
@@ -170,17 +170,18 @@ void
 js::InitMallocAllocator()
 {
     MallocArena = moz_create_arena();
 }
 
 void
 js::ShutDownMallocAllocator()
 {
-    moz_dispose_arena(MallocArena);
+    // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+    // moz_dispose_arena(MallocArena);
 }
 
 JS_PUBLIC_API(void)
 JS_Assert(const char* s, const char* file, int ln)
 {
     MOZ_ReportAssertionFailure(s, file, ln);
     MOZ_CRASH();
 }
--- a/memory/build/mozjemalloc.cpp
+++ b/memory/build/mozjemalloc.cpp
@@ -4560,16 +4560,20 @@ MozJemalloc::moz_create_arena_with_param
   }
   return 0;
 }
 
 template<>
 inline void
 MozJemalloc::moz_dispose_arena(arena_id_t aArenaId)
 {
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // We want to catch any such occurances of that behavior.
+  MOZ_CRASH("Do not call moz_dispose_arena until Bug 1364359 is fixed.");
+
   arena_t* arena = gArenas.GetById(aArenaId, /* IsPrivate = */ true);
   MOZ_RELEASE_ASSERT(arena);
   gArenas.DisposeArena(arena);
 }
 
 #define MALLOC_DECL(name, return_type, ...)                                    \
   template<>                                                                   \
   inline return_type MozJemalloc::moz_arena_##name(                            \
--- a/memory/gtest/TestJemalloc.cpp
+++ b/memory/gtest/TestJemalloc.cpp
@@ -266,32 +266,35 @@ TEST(Jemalloc, Arenas)
   void* ptr = moz_arena_malloc(arena, 42);
   ASSERT_TRUE(ptr != nullptr);
   ptr = moz_arena_realloc(arena, ptr, 64);
   ASSERT_TRUE(ptr != nullptr);
   moz_arena_free(arena, ptr);
   ptr = moz_arena_calloc(arena, 24, 2);
   // For convenience, free can be used to free arena pointers.
   free(ptr);
-  moz_dispose_arena(arena);
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // moz_dispose_arena(arena);
 
 #ifdef HAS_GDB_SLEEP_DURATION
   // Avoid death tests adding some unnecessary (long) delays.
   unsigned int old_gdb_sleep_duration = _gdb_sleep_duration;
   _gdb_sleep_duration = 0;
 #endif
 
   // Can't use an arena after it's disposed.
-  ASSERT_DEATH_WRAP(moz_arena_malloc(arena, 80), "");
+  // ASSERT_DEATH_WRAP(moz_arena_malloc(arena, 80), "");
 
   // Arena id 0 can't be used to somehow get to the main arena.
   ASSERT_DEATH_WRAP(moz_arena_malloc(0, 80), "");
 
   arena = moz_create_arena();
   arena_id_t arena2 = moz_create_arena();
+  // Ensure arena2 is used to prevent OSX errors:
+  (void)arena2;
 
   // For convenience, realloc can also be used to reallocate arena pointers.
   // The result should be in the same arena. Test various size class transitions.
   for (size_t from_size : sSizes) {
     SCOPED_TRACE(testing::Message() << "from_size = " << from_size);
     for (size_t to_size : sSizes) {
       SCOPED_TRACE(testing::Message() << "to_size = " << to_size);
       ptr = moz_arena_malloc(arena, from_size);
@@ -300,18 +303,20 @@ TEST(Jemalloc, Arenas)
       ASSERT_DEATH_WRAP(moz_arena_free(arena2, ptr), "");
       // Likewise for moz_arena_realloc.
       ASSERT_DEATH_WRAP(moz_arena_realloc(arena2, ptr, from_size), "");
       // The following will crash if it's not in the right arena.
       moz_arena_free(arena, ptr);
     }
   }
 
-  moz_dispose_arena(arena2);
-  moz_dispose_arena(arena);
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // moz_dispose_arena(arena2);
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // moz_dispose_arena(arena);
 
 #ifdef HAS_GDB_SLEEP_DURATION
   _gdb_sleep_duration = old_gdb_sleep_duration;
 #endif
 }
 
 // Check that a buffer aPtr is entirely filled with a given character from
 // aOffset to aSize. For faster comparison, the caller is required to fill a
@@ -424,17 +429,18 @@ TEST(Jemalloc, InPlace)
         EXPECT_EQ(ptr, ptr2);
       } else {
         EXPECT_NE(ptr, ptr2);
       }
       moz_arena_free(arena, ptr2);
     }
   }
 
-  moz_dispose_arena(arena);
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // moz_dispose_arena(arena);
 }
 
 TEST(Jemalloc, JunkPoison)
 {
   jemalloc_stats_t stats;
   jemalloc_stats(&stats);
 
   // Create buffers in a separate arena, for faster comparisons with
@@ -605,15 +611,17 @@ TEST(Jemalloc, JunkPoison)
           ASSERT_NO_FATAL_FAILURE(bulk_compare(
             ptr2, from_size, rounded_to_size, junk_buf, stats.page_size));
         }
         moz_arena_free(arena, ptr2);
       }
     }
   }
 
-  moz_dispose_arena(arena);
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // moz_dispose_arena(arena);
 
   moz_arena_free(buf_arena, poison_buf);
   moz_arena_free(buf_arena, junk_buf);
-  moz_dispose_arena(buf_arena);
+  // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
+  // moz_dispose_arena(buf_arena);
 }
 #endif