Bug 1550734 - Clarify the purpose and implementation of FreeOp::freeLater r=sfink DONTBUILD
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 10 May 2019 17:47:39 +0100
changeset 535330 d1e6799af0a63a17a08db30cbe86cb22ce585c43
parent 535329 34a413cfb0d225c4a2007b4d37bb63087d5a1436
child 535331 e96752781d2d3961d797cec48c5357c4ec293a23
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1550734
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1550734 - Clarify the purpose and implementation of FreeOp::freeLater r=sfink DONTBUILD Differential Revision: https://phabricator.services.mozilla.com/D30667
js/src/gc/FreeOp-inl.h
js/src/gc/FreeOp.h
--- a/js/src/gc/FreeOp-inl.h
+++ b/js/src/gc/FreeOp-inl.h
@@ -25,20 +25,22 @@ inline void FreeOp::freeLater(gc::Cell* 
                               MemoryUse use) {
   if (p) {
     RemoveCellMemory(cell, nbytes, use);
     freeLater(p);
   }
 }
 
 inline void FreeOp::freeLater(void* p) {
-  // FreeOps other than the defaultFreeOp() are constructed on the stack,
-  // and won't hold onto the pointers to free indefinitely.
+  // Default FreeOps are not constructed on the stack, and will hold onto the
+  // pointers to free indefinitely.
   MOZ_ASSERT(!isDefaultFreeOp());
 
+  // It's not safe to free this allocation immediately, so we must crash if we
+  // can't append to the list.
   AutoEnterOOMUnsafeRegion oomUnsafe;
   if (!freeLaterList.append(p)) {
     oomUnsafe.crash("FreeOp::freeLater");
   }
 }
 
 } // namespace js
 
--- a/js/src/gc/FreeOp.h
+++ b/js/src/gc/FreeOp.h
@@ -49,16 +49,24 @@ class FreeOp : public JSFreeOp {
 
   bool isDefaultFreeOp() const { return isDefault; }
 
   void free_(void* p) { js_free(p); }
 
   // Free memory that was associated with a GC thing using js::AddCellMemory.
   void free_(gc::Cell* cell, void* p, size_t nbytes, MemoryUse use);
 
+  // Queue an allocation to be freed when the FreeOp is destroyed.
+  //
+  // This should not be called on the default FreeOps returned by
+  // JSRuntime/JSContext::defaultFreeOp() since these are not destroyed until
+  // the runtime itself is destroyed.
+  //
+  // This is used to ensure that copy-on-write object elements are not freed
+  // until all objects that refer to them have been finalized.
   void freeLater(void* p);
 
   // Free memory that was associated with a GC thing using js::AddCellMemory.
   void freeLater(gc::Cell* cell, void* p, size_t nbytes, MemoryUse use);
 
   bool appendJitPoisonRange(const jit::JitPoisonRange& range) {
     // FreeOps other than the defaultFreeOp() are constructed on the stack,
     // and won't hold onto the pointers to free indefinitely.