Cleanup lifetime management for translation units

For PhpFiles, there was a mixed management system, where references from
translated code were held until the code was made unreachable, while
references from interpreted code were held for the duration of the
current request. Under the new scheme, PhpFiles are always treadmilled.
They are owned by the FileRepository, and so need to be ref-counted
because the FileRepository can have the same PhpFile under multiple
paths. But we don't ref-count the *uses* of PhpFiles anymore.

Classes still need to be ref-counted, but as soon as their Unit
goes away, most of their internals can be freed. We just need to
hold onto them if derived classes are referencing them. Even in
that case, the next time we try to instantiate the derived class,
we can kill any that reference this Class.

There were also lots of holes where references were not dropped,
or owned data structures were not destroyed. eg a Class's methods
were not destroyed when the Class was destroyed; there were
several paths where an entry was erased from the file map, but the
corresponding PhpFile was not decRef'd - or worse, a null entry
was left in the map (something we had asserts to check for).

This tries to make the handling more consistent.

hphp/runtime/base/execution_context.cpp

@@ -117,19 +117,7 @@ VMExecutionContext::~VMExecutionContext() {
        it != m_constInfo.end(); ++it) {
     delete it->second;
   }
-  // decRef all of the PhpFiles in m_evaledFiles. Any PhpFile whose refcount
-  // reaches zero will be destroyed. Currently each PhpFile "owns" its Unit,
-  // so when a PhpFile is destroyed it will free its Unit as well.
-  for (EvaledFilesMap::iterator it = m_evaledFiles.begin();
-       it != m_evaledFiles.end();) {
-    EvaledFilesMap::iterator current = it;
-    ++it;
-    StringData* sd = current->first;
-    Eval::PhpFile* efile = current->second;
-    efile->decRefAndDelete();
-    m_evaledFiles.erase(current);
-    decRefStr(sd);
-  }
+
   // Discard all units that were created via create_function().
   for (EvaledUnitsVec::iterator it = m_createdFuncs.begin();
        it != m_createdFuncs.end(); ++it) {

hphp/runtime/base/file_repository.cpp

@@ -28,6 +28,9 @@
 #include "hphp/runtime/vm/pendq.h"
 #include "hphp/runtime/vm/repo.h"
 #include "hphp/runtime/vm/runtime.h"
+#include "hphp/runtime/vm/treadmill.h"
+
+#include "folly/ScopeGuard.h"
 
 using std::endl;
 
@@ -75,8 +78,18 @@ int PhpFile::decRef(int n) {
 }
 
 void PhpFile::decRefAndDelete() {
+  class FileInvalidationTrigger : public Treadmill::WorkItem {
+    Eval::PhpFile* m_f;
+   public:
+    FileInvalidationTrigger(Eval::PhpFile* f) : m_f(f) { }
+    virtual void operator()() {
+      FileRepository::onDelete(m_f);
+    }
+  };
+
   if (decRef() == 0) {
-    FileRepository::onDelete(this);
+    if (RuntimeOption::EvalJit) Transl::Translator::Get()->invalidateFile(this);
+    Treadmill::WorkItem::enqueue(new FileInvalidationTrigger(this));
   }
 }
 
@@ -117,7 +130,7 @@ bool FileRepository::fileDump(const char *filename) {
   return true;
 }
 
-void FileRepository::onDelete(PhpFile *f) {
+void FileRepository::onDelete(PhpFile* f) {
   assert(f->getRef() == 0);
   if (md5Enabled()) {
     WriteLock lock(s_md5Lock);
@@ -155,41 +168,54 @@ PhpFile *FileRepository::checkoutFile(StringData *rname,
     if (s_files.find(acc, name.get()) && !acc->second->isChanged(s)) {
       TRACE(1, "FR fast path hit %s\n", rname->data());
       ret = acc->second->getPhpFile();
-      ret->incRef();
       return ret;
     }
   }
 
   TRACE(1, "FR fast path miss: %s\n", rname->data());
   const StringData *n = StringData::GetStaticString(name.get());
+
+  PhpFile* toKill = nullptr;
+  SCOPE_EXIT {
+    // run this after acc is destroyed (and its lock released)
+    if (toKill) toKill->decRefAndDelete();
+  };
   ParsedFilesMap::accessor acc;
   bool isNew = s_files.insert(acc, n);
-  assert(isNew || acc->second); // We don't leave null entries around.
-  bool isChanged = !isNew && acc->second->isChanged(s);
+  PhpFileWrapper* old = acc->second;
+  SCOPE_EXIT {
+    // run this just before acc is released
+    if (old && old != acc->second) {
+      if (old->getPhpFile() != acc->second->getPhpFile()) {
+        toKill = old->getPhpFile();
+      }
+      delete old;
+    }
+    if (!acc->second) s_files.erase(acc);
+  };
+
+  assert(isNew || old); // We don't leave null entries around.
+  bool isChanged = !isNew && old->isChanged(s);
 
   if (isNew || isChanged) {
     if (!readFile(n, s, fileInfo)) {
-      // Be sure to get rid of the new reference to it.
-      s_files.erase(acc);
       TRACE(1, "File disappeared between stat and FR::readNewFile: %s\n",
             rname->data());
       return nullptr;
     }
     ret = fileInfo.m_phpFile;
-    if (isChanged && ret == acc->second->getPhpFile()) {
+    if (isChanged && ret == old->getPhpFile()) {
       // The file changed but had the same contents.
       if (debug && md5Enabled()) {
         ReadLock lock(s_md5Lock);
         assert(s_md5Files.find(ret->getMd5())->second == ret);
       }
-      ret->incRef();
       return ret;
     }
   } else if (!isNew) {
     // Somebody might have loaded the file between when we dropped
     // our read lock and got the write lock
-    ret = acc->second->getPhpFile();
-    ret->incRef();
+    ret = old->getPhpFile();
     return ret;
   }
 
@@ -203,10 +229,7 @@ PhpFile *FileRepository::checkoutFile(StringData *rname,
       raise_error("Tried to parse %s in repo authoritative mode", n->data());
     }
     ret = parseFile(n->data(), fileInfo);
-    if (!ret) {
-      s_files.erase(acc);
-      return nullptr;
-    }
+    if (!ret) return nullptr;
   }
   assert(ret != nullptr);
 
@@ -215,22 +238,18 @@ PhpFile *FileRepository::checkoutFile(StringData *rname,
     ret->incRef();
     ret->setId(Transl::TargetCache::allocBit());
   } else {
-    PhpFile *f = acc->second->getPhpFile();
+    PhpFile *f = old->getPhpFile();
     if (f != ret) {
       ret->setId(f->getId());
-      Transl::Translator::Get()->invalidateFile(f); // f has changed
+      ret->incRef();
     }
-    f->decRefAndDelete();
-    delete acc->second;
     acc->second = new PhpFileWrapper(s, ret);
-    ret->incRef();
   }
 
   if (md5Enabled()) {
     WriteLock lock(s_md5Lock);
     s_md5Files[ret->getMd5()] = ret;
   }
-  ret->incRef();
   return ret;
 }
 

hphp/runtime/base/file_repository.h

@@ -121,7 +121,20 @@ typedef RankedCHM<const StringData*, HPHP::Eval::PhpFileWrapper*,
 typedef hphp_hash_map<std::string, PhpFile*, string_hash> Md5FileMap;
 
 /**
- * FileRepository is global.
+ * FileRepository tracks all the Units that are currently live in this session.
+ *
+ * Currently live means that its been loaded at least once, and we haven't
+ * noticed a change to the underlying file yet.
+ *
+ * FileRepository contains a chm from file-paths to PhpFileWrappers.
+ * The PhpFileWrapper is owned by the chm entry, and refers to a refCounted
+ * PhpFile (the refCount is the number of times it appears in the
+ * FileRepository in total - symlinks can cause a PhpFile to appear more than
+ * once). The PhpFile refers to, and owns a Unit.
+ *
+ * When the last reference to a PhpFile goes away, the Unit and its contents
+ * can no longer be reached by new requests, but existing requests could still
+ * be referring to it, so the Unit is freed via TreadMill.
  */
 class FileRepository {
 public:

hphp/runtime/base/object_data.cpp

@@ -363,12 +363,10 @@ void ObjectData::o_getArray(Array& props, bool pubOnly /* = false */) const {
   const Class* cls = m_cls;
   do {
     getProps(cls, pubOnly, cls->m_preClass.get(), props, inserted);
-    auto& usedTraits = cls->m_usedTraits;
-    for (unsigned t = 0; t < usedTraits.size(); t++) {
-      const ClassPtr& trait = usedTraits[t];
+    for (auto const& trait : cls->m_usedTraits) {
       getProps(cls, pubOnly, trait->m_preClass.get(), props, inserted);
     }
-    cls = cls->m_parent.get();
+    cls = cls->parent();
   } while (cls);
 
   // Iterate over dynamic properties and insert {name --> prop} pairs.
@@ -419,7 +417,7 @@ Array ObjectData::o_toIterArray(CStrRef context,
         }
       }
     }
-    klass = klass->m_parent.get();
+    klass = klass->parent();
   }
 
   // Now get dynamic properties.

hphp/runtime/ext/ext_reflection.cpp

@@ -759,14 +759,11 @@ Array f_hphp_get_class_info(CVarRef name) {
   ret.set(s_extension, empty_string);
   ret.set(s_parent,    cls->parentRef());
 
-  typedef vector<ClassPtr> ClassVec;
   // interfaces
   {
     Array arr = Array::Create();
-    const ClassVec &interfaces = cls->declInterfaces();
-    for (ClassVec::const_iterator it = interfaces.begin(),
-           end = interfaces.end(); it != end; ++it) {
-      arr.set(it->get()->nameRef(), VarNR(1));
+    for (auto const& interface : cls->declInterfaces()) {
+      arr.set(interface->nameRef(), VarNR(1));
     }
     ret.set(s_interfaces, VarNR(arr));
   }
@@ -774,10 +771,8 @@ Array f_hphp_get_class_info(CVarRef name) {
   // traits
   {
     Array arr = Array::Create();
-    const ClassVec &traits = cls->usedTraits();
-    for (ClassVec::const_iterator it = traits.begin(),
-           end = traits.end(); it != end; ++it) {
-      arr.set(it->get()->nameRef(), VarNR(1));
+    for (auto const& trait : cls->usedTraits()) {
+      arr.set(trait->nameRef(), VarNR(1));
     }
     ret.set(s_traits, VarNR(arr));
   }

hphp/runtime/ext/ext_spl.cpp

@@ -195,7 +195,7 @@ Variant f_class_uses(CVarRef obj, bool autoload /* = true */) {
   }
   Array ret(Array::Create());
   for (auto& elem : cls->usedTraits()) {
-    auto& traitName = elem.get()->nameRef();
+    auto& traitName = elem->nameRef();
     ret.set(traitName, traitName);
   }
   return ret;

hphp/runtime/vm/bytecode.cpp

@@ -2287,7 +2287,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
     // We found it! Return the unit.
     efile = it->second;
     initial = false;
-    if (!initial_opt) efile->incRef();
     return efile;
   }
   // We didn't find it, so try the realpath.
@@ -2309,9 +2308,7 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
           efile = it->second;
           m_evaledFiles[spath.get()] = efile;
           spath.get()->incRefCount();
-          efile->incRef();
           initial = false;
-          if (!initial_opt) efile->incRef();
           return efile;
         }
       }
@@ -2320,7 +2317,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
   // This file hasn't been included yet, so we need to parse the file
   efile = HPHP::Eval::FileRepository::checkoutFile(
     hasRealpath ? rpath.get() : spath.get(), s);
-  assert(!efile || efile->getRef() > 0);
   if (efile && initial_opt) {
     // if initial_opt is not set, this shouldnt be recorded as a
     // per request fetch of the file.
@@ -2335,7 +2331,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
     if (hasRealpath) {
       m_evaledFiles[rpath.get()] = efile;
       rpath.get()->incRefCount();
-      efile->incRef();
     }
     DEBUGGER_ATTACHED_ONLY(phpDebuggerFileLoadHook(efile));
   }
@@ -2393,7 +2388,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupIncludeRoot(StringData* path,
   EvaledFilesMap::const_iterator it = m_evaledFiles.find(absPath.get());
   if (it != m_evaledFiles.end()) {
     if (initial) *initial = false;
-    if (!initial) it->second->incRef();
     return it->second;
   }
 

hphp/runtime/vm/class.cpp

@@ -195,11 +195,11 @@ void PreClass::prettyPrint(std::ostream &out) const {
 //=============================================================================
 // Class.
 
-ClassPtr Class::newClass(PreClass* preClass, Class* parent) {
+Class* Class::newClass(PreClass* preClass, Class* parent) {
   unsigned classVecLen = (parent != nullptr) ? parent->m_classVecLen+1 : 1;
   void* mem = Util::low_malloc(sizeForNClasses(classVecLen));
   try {
-    return ClassPtr(new (mem) Class(preClass, parent, classVecLen));
+    return new (mem) Class(preClass, parent, classVecLen);
   } catch (...) {
     Util::low_free(mem);
     throw;
@@ -207,7 +207,7 @@ ClassPtr Class::newClass(PreClass* preClass, Class* parent) {
 }
 
 Class::Class(PreClass* preClass, Class* parent, unsigned classVecLen)
-  : m_preClass(PreClassPtr(preClass)), m_parent(ClassPtr(parent)),
+  : m_preClass(PreClassPtr(preClass)), m_parent(parent),
     m_traitsBeginIdx(0), m_traitsEndIdx(0), m_clsInfo(nullptr),
     m_builtinPropSize(0), m_classVecLen(classVecLen), m_cachedOffset(0),
     m_propDataCache(-1), m_propSDataCache(-1), m_InstanceCtor(nullptr),
@@ -224,26 +224,99 @@ Class::Class(PreClass* preClass, Class* parent, unsigned classVecLen)
   setClassVec();
 }
 
-void Class::atomicRelease() {
-  if (m_cachedOffset != 0u) {
-    /*
-      m_cachedOffset is initialied to 0, and is only set
-      when the Class is put on the list, so we only have to
-      remove this node if its NOT 0.
-      Since we're about to remove it, reset to 0 so we know
-      its safe to kill the node during the delayed Treadmill
-      callback.
-    */
-    m_cachedOffset = 0u;
-    PreClass* pcls = m_preClass.get();
-    {
-      Lock l(Unit::s_classesMutex);
-      pcls->namedEntity()->removeClass(this);
+Class::~Class() {
+  releaseRefs();
+
+  auto methods = methodRange();
+  while (!methods.empty()) {
+    Func* meth = methods.popFront();
+    if (meth) Func::destroy(meth);
+  }
+}
+
+void Class::releaseRefs() {
+  /*
+   * We have to be careful here.
+   * We want to free up as much as possible as early as possible, but
+   * some of our methods may actually belong to our parent
+   * This means we can't destroy *our* Funcs until our refCount
+   * hits zero (ie when Class::~Class gets called), because there
+   * could be a child class which hasn't yet been destroyed, which
+   * will need to inspect them. Also, we need to inspect the Funcs
+   * now (while we still have a references to parent) to determine
+   * which ones we will eventually need to free.
+   * Similarly, if any of our func's belong to a parent class, we
+   * can't free the parent, because one of our children could also
+   * have a reference to those func's (and its only reference to
+   * our parent is via this class).
+   */
+  auto methods = mutableMethodRange();
+  bool okToReleaseParent = true;
+  while (!methods.empty()) {
+    Func*& meth = methods.popFront();
+    if (meth /* releaseRefs can be called more than once */ &&
+        meth->cls() != this &&
+        ((meth->attrs() & AttrPrivate) || !meth->hasStaticLocals())) {
+      meth = nullptr;
+      okToReleaseParent = false;
     }
-    Treadmill::WorkItem::enqueue(new Treadmill::FreeClassTrigger(this));
-    return;
   }
 
+  if (okToReleaseParent) {
+    m_parent.reset();
+  }
+  m_declInterfaces.clear();
+  m_usedTraits.clear();
+}
+
+namespace {
+
+class FreeClassTrigger : public Treadmill::WorkItem {
+  TRACE_SET_MOD(treadmill);
+  Class* m_cls;
+ public:
+  explicit FreeClassTrigger(Class* cls) : m_cls(cls) {
+    TRACE(3, "FreeClassTrigger @ %p, cls %p\n", this, m_cls);
+  }
+  void operator()() {
+    TRACE(3, "FreeClassTrigger: Firing @ %p , cls %p\n", this, m_cls);
+    if (!m_cls->decAtomicCount()) {
+      m_cls->atomicRelease();
+    }
+  }
+};
+
+}
+
+void Class::destroy() {
+  /*
+   * If we were never put on NamedEntity::classList, or
+   * we've already been destroy'd, there's nothing to do
+   */
+  if (!m_cachedOffset) return;
+
+  Lock l(Unit::s_classesMutex);
+  // Need to recheck now we have the lock
+  if (!m_cachedOffset) return;
+  // Only do this once.
+  m_cachedOffset = 0;
+
+  PreClass* pcls = m_preClass.get();
+  pcls->namedEntity()->removeClass(this);
+  /*
+   * Regardless of refCount, this Class is now unusable.
+   * Release what we can immediately, to allow dependent
+   * classes to be freed.
+   * Needs to be under the lock, because multiple threads
+   * could call destroy
+   */
+  releaseRefs();
+  Treadmill::WorkItem::enqueue(new FreeClassTrigger(this));
+}
+
+void Class::atomicRelease() {
+  assert(!m_cachedOffset);
+  assert(!getCount());
   this->~Class();
   Util::low_free(this);
 }
@@ -262,16 +335,13 @@ bool Class::verifyPersistent() const {
       !Transl::TargetCache::isPersistentHandle(m_parent->m_cachedOffset)) {
     return false;
   }
-  for (size_t i = 0, nInterfaces = m_declInterfaces.size();
-       i < nInterfaces; ++i) {
-    Class* declInterface = m_declInterfaces[i].get();
+  for (auto const& declInterface : m_declInterfaces) {
     if (!Transl::TargetCache::isPersistentHandle(
           declInterface->m_cachedOffset)) {
       return false;
     }
   }
-  for (size_t i = 0; i < m_usedTraits.size(); i++) {
-    Class* usedTrait = m_usedTraits[i].get();
+  for (auto const& usedTrait : m_usedTraits) {
     if (!Transl::TargetCache::isPersistentHandle(
           usedTrait->m_cachedOffset)) {
       return false;
@@ -429,11 +499,15 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
         return Avail::Fail;
       }
     }
-    if (parent != ourParent) return Avail::False;
+    if (parent != ourParent) {
+      if (UNLIKELY(ourParent->isZombie())) {
+        const_cast<Class*>(this)->destroy();
+      }
+      return Avail::False;
+    }
   }
-  for (size_t i = 0, nInterfaces = m_declInterfaces.size();
-       i < nInterfaces; ++i) {
-    Class* declInterface = m_declInterfaces[i].get();
+  for (auto const& di : m_declInterfaces) {
+    Class* declInterface = di.get();
     PreClass *pint = declInterface->m_preClass.get();
     Class* interface = Unit::getClass(pint->namedEntity(), pint->name(),
                                       tryAutoload);
@@ -442,11 +516,14 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
         parent = declInterface;
         return Avail::Fail;
       }
+      if (UNLIKELY(declInterface->isZombie())) {
+        const_cast<Class*>(this)->destroy();
+      }
       return Avail::False;
     }
   }
-  for (size_t i = 0; i < m_usedTraits.size(); i++) {
-    Class* usedTrait = m_usedTraits[i].get();
+  for (auto const& ut : m_usedTraits) {
+    Class* usedTrait = ut.get();
     PreClass* ptrait = usedTrait->m_preClass.get();
     Class* trait = Unit::getClass(ptrait->namedEntity(), ptrait->name(),
                                   tryAutoload);
@@ -455,6 +532,9 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
         parent = usedTrait;
         return Avail::Fail;
       }
+      if (UNLIKELY(usedTrait->isZombie())) {
+        const_cast<Class*>(this)->destroy();
+      }
       return Avail::False;
     }
   }
@@ -1020,26 +1100,26 @@ void Class::applyTraitPrecRule(const PreClass::TraitPrecRule& rule,
   }
 }
 
-ClassPtr Class::findSingleTraitWithMethod(const StringData* methName) {
+Class* Class::findSingleTraitWithMethod(const StringData* methName) {
   // Note: m_methods includes methods from parents / traits recursively
-  ClassPtr traitCls = ClassPtr();
-  for (size_t t = 0; t < m_usedTraits.size(); t++) {
-    if (m_usedTraits[t]->m_methods.contains(methName)) {
-      if (traitCls.get() != nullptr) { // more than one trait contains method
-        return ClassPtr();
+  Class* traitCls = nullptr;
+  for (auto const& t : m_usedTraits) {
+    if (t->m_methods.contains(methName)) {
+      if (traitCls != nullptr) { // more than one trait contains method
+        return nullptr;
       }
-      traitCls = m_usedTraits[t];
+      traitCls = t.get();
     }
   }
   return traitCls;
 }
 
 void Class::setImportTraitMethodModifiers(TraitMethodList& methList,
-                                          ClassPtr         traitCls,
+                                          Class*           traitCls,
                                           Attr             modifiers) {
   for (TraitMethodList::iterator iter = methList.begin();
        iter != methList.end(); iter++) {
-    if (iter->m_trait.get() == traitCls.get()) {
+    if (iter->m_trait == traitCls) {
       iter->m_modifiers = modifiers;
       return;
     }
@@ -1065,14 +1145,14 @@ void Class::applyTraitAliasRule(const PreClass::TraitAliasRule& rule,
   const StringData* origMethName = rule.getOrigMethodName();
   const StringData* newMethName  = rule.getNewMethodName();
 
-  ClassPtr traitCls;
+  Class* traitCls = nullptr;
   if (traitName->empty()) {
     traitCls = findSingleTraitWithMethod(origMethName);
   } else {
     traitCls = Unit::loadClass(traitName);
   }
 
-  if (!traitCls.get() || (!(traitCls->attrs() & AttrTrait))) {
+  if (!traitCls || (!(traitCls->attrs() & AttrTrait))) {
     raise_error("unknown trait '%s'", traitName->data());
   }
 
@@ -1115,7 +1195,6 @@ void Class::applyTraitRules(MethodToTraitListMap& importMethToTraitMap) {
 void Class::importTraitMethod(const TraitMethod&  traitMethod,
                               const StringData*   methName,
                               MethodMap::Builder& builder) {
-  ClassPtr trait     = traitMethod.m_trait;
   Func*    method    = traitMethod.m_method;
   Attr     modifiers = traitMethod.m_modifiers;
 
@@ -1218,8 +1297,8 @@ void Class::importTraitMethods(MethodMap::Builder& builder) {
   MethodToTraitListMap importMethToTraitMap;
 
   // 1. Find all methods to be imported
-  for (size_t t = 0; t < m_usedTraits.size(); t++) {
-    ClassPtr trait = m_usedTraits[t];
+  for (auto const& t : m_usedTraits) {
+    Class* trait = t.get();
     for (Slot i = 0; i < trait->m_methods.size(); ++i) {
       Func* method = trait->m_methods[i];
       const StringData* methName = method->name();
@@ -1460,8 +1539,7 @@ void Class::setConstants() {
   }
 
   // Copy in interface constants.
-  for (std::vector<ClassPtr>::const_iterator it = m_declInterfaces.begin();
-       it != m_declInterfaces.end(); ++it) {
+  for (auto it = m_declInterfaces.begin(); it != m_declInterfaces.end(); ++it) {
     for (Slot slot = 0; slot < (*it)->m_constants.size(); ++slot) {
       const Const& iConst = (*it)->m_constants[slot];
 
@@ -1763,7 +1841,7 @@ bool Class::compatibleTraitPropInit(TypedValue& tv1, TypedValue& tv2) {
   }
 }
 
-void Class::importTraitInstanceProp(ClassPtr    trait,
+void Class::importTraitInstanceProp(Class*      trait,
                                     Prop&       traitProp,
                                     TypedValue& traitPropVal,
                                     PropMap::Builder& curPropMap) {
@@ -1793,7 +1871,7 @@ void Class::importTraitInstanceProp(ClassPtr    trait,
   }
 }
 
-void Class::importTraitStaticProp(ClassPtr trait,
+void Class::importTraitStaticProp(Class*   trait,
                                   SProp&   traitProp,
                                   PropMap::Builder& curPropMap,
                                   SPropMap::Builder& curSPropMap) {
@@ -1836,8 +1914,8 @@ void Class::importTraitStaticProp(ClassPtr trait,
 void Class::importTraitProps(PropMap::Builder& curPropMap,
                              SPropMap::Builder& curSPropMap) {
   if (attrs() & AttrNoExpandTrait) return;
-  for (size_t t = 0; t < m_usedTraits.size(); t++) {
-    ClassPtr trait = m_usedTraits[t];
+  for (auto& t : m_usedTraits) {
+    Class* trait = t.get();
 
     // instance properties
     for (Slot p = 0; p < trait->m_declProperties.size(); p++) {
@@ -1859,7 +1937,7 @@ void Class::importTraitProps(PropMap::Builder& curPropMap,
 void Class::addTraitPropInitializers(bool staticProps) {
   if (attrs() & AttrNoExpandTrait) return;
   for (unsigned t = 0; t < m_usedTraits.size(); t++) {
-    ClassPtr trait = m_usedTraits[t];
+    Class* trait = m_usedTraits[t].get();
     InitVec& traitInitVec = staticProps ? trait->m_sinitVec : trait->m_pinitVec;
     InitVec& thisInitVec  = staticProps ? m_sinitVec : m_pinitVec;
     // Insert trait's 86[ps]init into the current class, avoiding repetitions.
@@ -1974,17 +2052,17 @@ void Class::setInterfaces() {
   for (PreClass::InterfaceVec::const_iterator it =
          m_preClass->interfaces().begin();
        it != m_preClass->interfaces().end(); ++it) {
-    auto cp = ClassPtr(Unit::loadClass(*it));
-    if (cp.get() == nullptr) {
+    Class* cp = Unit::loadClass(*it);
+    if (cp == nullptr) {
       raise_error("Undefined interface: %s", (*it)->data());
     }
     if (!(cp->attrs() & AttrInterface)) {
       raise_error("%s cannot implement %s - it is not an interface",
                   m_preClass->name()->data(), cp->name()->data());
     }
-    m_declInterfaces.push_back(cp);
+    m_declInterfaces.push_back(ClassPtr(cp));
     if (interfacesBuilder.find(cp->name()) == interfacesBuilder.end()) {
-      interfacesBuilder.add(cp->name(), cp.get());
+      interfacesBuilder.add(cp->name(), cp);
     }
     int size = cp->m_interfaces.size();
     for (int i = 0; i < size; i++) {
@@ -2004,8 +2082,8 @@ void Class::setUsedTraits() {
   for (PreClass::UsedTraitVec::const_iterator
        it = m_preClass->usedTraits().begin();
        it != m_preClass->usedTraits().end(); it++) {
-    auto classPtr = ClassPtr(Unit::loadClass(*it));
-    if (classPtr.get() == nullptr) {
+    Class* classPtr = Unit::loadClass(*it);
+    if (classPtr == nullptr) {
       raise_error("Trait '%s' not found", (*it)->data());
     }
     if (!(classPtr->attrs() & AttrTrait)) {
@@ -2013,14 +2091,14 @@ void Class::setUsedTraits() {
                   m_preClass->name()->data(),
                   classPtr->name()->data());
     }
-    m_usedTraits.push_back(classPtr);
+    m_usedTraits.push_back(ClassPtr(classPtr));
   }
 }
 
 void Class::setClassVec() {
   if (m_classVecLen > 1) {
     assert(m_parent.get() != nullptr);
-    memcpy(m_classVec, m_parent.get()->m_classVec,
+    memcpy(m_classVec, m_parent->m_classVec,
            (m_classVecLen-1) * sizeof(Class*));
   }
   m_classVec[m_classVecLen-1] = this;
@@ -2041,12 +2119,12 @@ void Class::setInstanceBitsImpl() {
 
   InstanceBits bits;
   bits.set(0);
-  auto setBits = [&](ClassPtr& c) {
+  auto setBits = [&](Class* c) {
     if (setParents) c->setInstanceBitsAndParents();
     bits |= c->m_instanceBits;
   };
-  if (m_parent.get()) setBits(m_parent);
-  std::for_each(m_declInterfaces.begin(), m_declInterfaces.end(), setBits);
+  if (m_parent.get()) setBits(m_parent.get());
+  for (auto& di : m_declInterfaces) setBits(di.get());
 
   unsigned bit;
   if (mapGet(s_instanceBits, m_preClass->name(), &bit)) {
@@ -2082,9 +2160,9 @@ void Class::getMethodNames(const Class* ctx, HphpArray* methods) const {
     }
     methods->set(const_cast<StringData*>(func->name()), true_varNR, false);
   }
-  if (m_parent.get()) m_parent.get()->getMethodNames(ctx, methods);
+  if (m_parent.get()) m_parent->getMethodNames(ctx, methods);
   for (int i = 0, sz = m_declInterfaces.size(); i < sz; i++) {
-    m_declInterfaces[i].get()->getMethodNames(ctx, methods);
+    m_declInterfaces[i]->getMethodNames(ctx, methods);
   }
 }
 

hphp/runtime/vm/class.h

@@ -446,9 +446,40 @@ public:
 
 public:
   // Call newClass() instead of directly calling new.
-  static ClassPtr newClass(PreClass* preClass, Class* parent);
+  static Class* newClass(PreClass* preClass, Class* parent);
   Class(PreClass* preClass, Class* parent, unsigned classVecLen);
+  ~Class();
+  /*
+   * destroy() is called when a Class becomes unreachable. This may happen
+   * before its refCount hits zero, because it is referred to by
+   * its NamedEntity, any derived classes, and (for interfaces) and Class
+   * that implents it, and (for traits) any class that uses it.
+   * Such referring classes must also be logically dead at the time
+   * destroy is called, but we dont have back pointers to find them,
+   * so instead we leave the Class in a zombie state. When we try to
+   * instantiate one of its referrers, we will notice that it depends
+   * on a zombie, and destroy *that*, releasing its refernce to this
+   * Class.
+   */
+  void destroy();
+  /*
+   * atomicRelease() is called when the (atomic) refCount hits zero.
+   * The class is completely dead at this point, and its memory is
+   * freed immediately.
+   */
   void atomicRelease();
+  /*
+   * releaseRefs() is called when a Class is put into the zombie state,
+   * to free any references to child classes, interfaces and traits
+   * Its safe to call multiple times, so is also called from the destructor
+   * (in case we bypassed the zombie state).
+   */
+  void releaseRefs();
+  /*
+   * isZombie() returns true if this class has been logically destroyed,
+   * but needed to be preserved due to outstanding references.
+   */
+  bool isZombie() const { return !m_cachedOffset; }
 
   static size_t sizeForNClasses(unsigned nClasses) {
     return offsetof(Class, m_classVec) + (sizeof(Class*) * nClasses);
@@ -693,10 +724,10 @@ private:
   static std::atomic<bool> s_instanceBitsInit;
 
   struct TraitMethod {
-    ClassPtr          m_trait;
+    Class*            m_trait;
     Func*             m_method;
     Attr              m_modifiers;
-    TraitMethod(ClassPtr trait, Func* method, Attr modifiers) :
+    TraitMethod(Class* trait, Func* method, Attr modifiers) :
         m_trait(trait), m_method(method), m_modifiers(modifiers) { }
   };
 
@@ -718,9 +749,9 @@ private:
   void importTraitMethod(const TraitMethod&  traitMethod,
                          const StringData*   methName,
                          MethodMap::Builder& curMethodMap);
-  ClassPtr findSingleTraitWithMethod(const StringData* methName);
+  Class* findSingleTraitWithMethod(const StringData* methName);
   void setImportTraitMethodModifiers(TraitMethodList& methList,
-                                     ClassPtr         traitCls,
+                                     Class*           traitCls,
                                      Attr             modifiers);
   void importTraitMethods(MethodMap::Builder& curMethodMap);
   void addTraitPropInitializers(bool staticProps);
@@ -731,11 +762,11 @@ private:
                            MethodToTraitListMap& importMethToTraitMap);
   void importTraitProps(PropMap::Builder& curPropMap,
                         SPropMap::Builder& curSPropMap);
-  void importTraitInstanceProp(ClassPtr    trait,
+  void importTraitInstanceProp(Class*      trait,
                                Prop&       traitProp,
                                TypedValue& traitPropVal,
                                PropMap::Builder& curPropMap);
-  void importTraitStaticProp(ClassPtr trait,
+  void importTraitStaticProp(Class*   trait,
                              SProp&   traitProp,
                              PropMap::Builder& curPropMap,
                              SPropMap::Builder& curSPropMap);

hphp/runtime/vm/fixed_string_map.cpp

@@ -38,10 +38,10 @@ inline bool strEqual(bool case_sensitive,
 }
 
 template <typename V, bool case_sensitive>
-FixedStringMap<V, case_sensitive>::~FixedStringMap() {
-  if (m_table != (Elm*)&null_key) {
-    free(m_table);
-  }
+void FixedStringMap<V, case_sensitive>::clear() {
+  if (m_table != (Elm*)&null_key) free(m_table);
+  m_table = nullptr;
+  m_mask = 0;
 }
 
 template <typename V, bool case_sensitive>

hphp/runtime/vm/fixed_string_map.h

@@ -27,8 +27,8 @@ template <typename V, bool case_sensitive> class FixedStringMap {
 public:
   explicit FixedStringMap(int num) : m_table(0) { init(num); }
   FixedStringMap() : m_mask(0), m_table(0) {}
-  ~FixedStringMap();
-
+  ~FixedStringMap() { clear(); }
+  void clear();
   void init(int num);
   void add(const StringData* s, const V& v);
   V *find(const StringData* s) const;

hphp/runtime/vm/func.cpp

@@ -238,6 +238,9 @@ void Func::destroy(Func* func) {
   if (func->isClosureBody() || func->isGeneratorFromClosure()) {
     Func** startOfFunc = (Func**) mem;
     mem = startOfFunc - 1; // move back by a pointer
+    if (Func* f = startOfFunc[-1]) {
+      Func::destroy(f);
+    }
   }
   func->~Func();
   Util::low_free(mem);

hphp/runtime/vm/indexed_string_map.h

@@ -49,6 +49,13 @@ struct IndexedStringMap {
     delete [] m_vec;
   }
 
+  void clear() {
+    delete [] m_vec;
+    m_vec = nullptr;
+    m_map.clear();
+    m_size = 0;
+  }
+
   /*
    * Create an IndexedStringMap from the supplied builder.  See
    * builder below.

hphp/runtime/vm/jit/codegen.cpp

@@ -2347,6 +2347,10 @@ void CodeGenerator::cgLdRetAddr(IRInstruction* inst) {
 
 void checkFrame(ActRec* fp, Cell* sp, bool checkLocals) {
   const Func* func = fp->m_func;
+  func->validate();
+  if (func->cls()) {
+    assert(!func->cls()->isZombie());
+  }
   if (fp->hasVarEnv()) {
     assert(fp->getVarEnv()->getCfp() == fp);
   }

hphp/runtime/vm/jit/translator-x64.cpp

@@ -4047,6 +4047,4 @@ void TranslatorX64::invalidateSrcKey(SrcKey sk) {
 
 } // HPHP::Transl
 
-TRACE_SET_MOD(tx64);
-
 } // HPHP

hphp/runtime/vm/jit/translator.cpp

@@ -4005,16 +4005,6 @@ void Translator::setTransCounter(TransID transId, uint64_t value) {
 
 namespace {
 
-struct DeferredFileInvalidate : public DeferredWorkItem {
-  Eval::PhpFile* m_f;
-  explicit DeferredFileInvalidate(Eval::PhpFile* f) : m_f(f) {
-    TRACE(2, "DeferredFileInvalidate @ %p, m_f %p\n", this, m_f); }
-  void operator()() {
-    TRACE(2, "DeferredFileInvalidate: Firing @ %p , m_f %p\n", this, m_f);
-    tx64->invalidateFileWork(m_f);
-  }
-};
-
 struct DeferredPathInvalidate : public DeferredWorkItem {
   const std::string m_path;
   explicit DeferredPathInvalidate(const std::string& path) : m_path(path) {
@@ -4031,49 +4021,14 @@ struct DeferredPathInvalidate : public DeferredWorkItem {
      * this lookup; since the path has changed, the file we'll get out is
      * going to be some new file, not the old file that needs invalidation.
      */
-    UNUSED Eval::PhpFile* f =
-      g_vmContext->lookupPhpFile(spath.get(), "");
-    // We don't keep around the extra ref.
-    if (f) f->decRefAndDelete();
+    (void)g_vmContext->lookupPhpFile(spath.get(), "");
   }
 };
 
 }
 
-void Translator::invalidateFileWork(Eval::PhpFile* f) {
-  class FileInvalidationTrigger : public Treadmill::WorkItem {
-    Eval::PhpFile* m_f;
-    int m_nRefs;
-  public:
-    FileInvalidationTrigger(Eval::PhpFile* f, int n) : m_f(f), m_nRefs(n) { }
-    virtual void operator()() {
-      if (m_f->decRef(m_nRefs) == 0) {
-        Eval::FileRepository::onDelete(m_f);
-      }
-    }
-  };
-  size_t nSmashed = tx64->m_srcDB.invalidateCode(f);
-  if (nSmashed) {
-    // The srcDB found an entry for this file. The entry's dependency
-    // on this file was counted as a reference, and the code is no longer
-    // reachable. We need to wait until the last outstanding request
-    // drains to know that we can really remove the reference.
-    Treadmill::WorkItem::enqueue(new FileInvalidationTrigger(f, nSmashed));
-  }
-}
-
-bool Translator::invalidateFile(Eval::PhpFile* f) {
-  // This is called from high rank, but we'll need the write lease to
-  // invalidate code.
-  if (!RuntimeOption::EvalJit) return false;
-  assert(f != nullptr);
-  PendQ::defer(new DeferredFileInvalidate(f));
-  return true;
-}
-
-void invalidatePath(const std::string& path) {
-  TRACE(1, "invalidatePath: abspath %s\n", path.c_str());
-  PendQ::defer(new DeferredPathInvalidate(path));
+void Translator::invalidateFile(Eval::PhpFile* f) {
+  m_srcDB.invalidateCode(f);
 }
 
 static const char *transKindStr[] = {

hphp/runtime/vm/jit/translator.h

@@ -959,8 +959,7 @@ public:
   }
 
   // Async hook for file modifications.
-  bool invalidateFile(Eval::PhpFile* f);
-  void invalidateFileWork(Eval::PhpFile* f);
+  void invalidateFile(Eval::PhpFile* f);
 
   // Start a new translation space. Returns true IFF this thread created
   // a new space.

hphp/runtime/vm/treadmill.cpp

@@ -26,7 +26,6 @@
 #include "hphp/util/base.h"
 #include "hphp/util/rank.h"
 #include "hphp/runtime/base/macros.h"
-#include "hphp/runtime/base/complex_types.h"
 #include "hphp/runtime/vm/jit/translator-x64.h"
 
 namespace HPHP {  namespace Treadmill {
@@ -141,15 +140,6 @@ void FreeMemoryTrigger::operator()() {
   free(m_ptr);
 }
 
-FreeClassTrigger::FreeClassTrigger(Class* cls) : m_cls(cls) {
-  TRACE(3, "FreeClassTrigger @ %p, cls %p\n", this, m_cls);
-}
-
-void FreeClassTrigger::operator()() {
-  TRACE(3, "FreeClassTrigger: Firing @ %p , cls %p\n", this, m_cls);
-  m_cls->atomicRelease();
-}
-
 void deferredFree(void* p) {
   WorkItem::enqueue(new FreeMemoryTrigger(p));
 }

hphp/runtime/vm/treadmill.h

@@ -59,14 +59,7 @@ class WorkItem {
 class FreeMemoryTrigger : public WorkItem {
   void* m_ptr;
  public:
-  FreeMemoryTrigger(void* ptr);
-  virtual void operator()();
-};
-
-class FreeClassTrigger : public Treadmill::WorkItem {
-  Class* m_cls;
- public:
-  FreeClassTrigger(Class* cls);
+  explicit FreeMemoryTrigger(void* ptr);
   virtual void operator()();
 };
 

hphp/runtime/vm/unit.cpp

@@ -413,9 +413,7 @@ Unit::~Unit() {
       Class* cur = cls;
       cls = cls->m_nextClass;
       if (cur->preClass() == pcls) {
-        if (!cur->decAtomicCount()) {
-          cur->atomicRelease();
-        }
+        cur->destroy();
       }
     }
   }
@@ -559,14 +557,15 @@ Class* Unit::defClass(const PreClass* preClass,
     // Search for a compatible extant class.  Searching from most to least
     // recently created may have better locality than alternative search orders.
     // In addition, its the only simple way to make this work lock free...
-    for (Class* class_ = top; class_ != nullptr; class_ = class_->m_nextClass) {
-      if (class_->preClass() != preClass) continue;
-
-      Class::Avail avail = class_->avail(parent, failIsFatal /*tryAutoload*/);
+    for (Class* class_ = top; class_ != nullptr; ) {
+      Class* cur = class_;
+      class_ = class_->m_nextClass;
+      if (cur->preClass() != preClass) continue;
+      Class::Avail avail = cur->avail(parent, failIsFatal /*tryAutoload*/);
       if (LIKELY(avail == Class::Avail::True)) {
-        class_->setCached();
-        DEBUGGER_ATTACHED_ONLY(phpDebuggerDefClassHook(class_));
-        return class_;
+        cur->setCached();
+        DEBUGGER_ATTACHED_ONLY(phpDebuggerDefClassHook(cur));
+        return cur;
       }
       if (avail == Class::Avail::Fail) {
         if (failIsFatal) {
@@ -597,9 +596,6 @@ Class* Unit::defClass(const PreClass* preClass,
     }
     Lock l(Unit::s_classesMutex);
 
-    /*
-      We could re-enter via Unit::getClass() or class_->avail().
-    */
     if (UNLIKELY(top != nameList->clsList())) {
       top = nameList->clsList();
       continue;
@@ -610,24 +606,25 @@ Class* Unit::defClass(const PreClass* preClass,
     }
     newClass->m_cachedOffset = nameList->m_cachedClassOffset;
 
+    newClass.get()->incAtomicCount();
+    newClass.get()->setCached();
     if (Class::s_instanceBitsInit.load(std::memory_order_acquire)) {
-      // If the instance bitmap has already been set up, we can just initialize
-      // our new class's bits and add ourselves to the class list normally.
+      // If the instance bitmap has already been set up, we can just
+      // initialize our new class's bits and add ourselves to the class
+      // list normally.
       newClass->setInstanceBits();
       nameList->pushClass(newClass.get());
     } else {
       // Otherwise, we have to grab the read lock. If the map has been
-      // initialized since we checked, initialize the bits normally. If not, we
-      // must add the new class to the class list before dropping the lock to
-      // ensure its bits are initialized when the time comes.
+      // initialized since we checked, initialize the bits normally. If not,
+      // we must add the new class to the class list before dropping the lock
+      // to ensure its bits are initialized when the time comes.
       ReadLock l(Class::s_instanceBitsLock);
       if (Class::s_instanceBitsInit.load(std::memory_order_acquire)) {
         newClass->setInstanceBits();
       }
       nameList->pushClass(newClass.get());
     }
-    newClass.get()->incAtomicCount();
-    newClass.get()->setCached();
     DEBUGGER_ATTACHED_ONLY(phpDebuggerDefClassHook(newClass.get()));
     return newClass.get();
   }