diff --git a/hphp/runtime/base/execution_context.cpp b/hphp/runtime/base/execution_context.cpp index f4c5a73be..0d0d02409 100644 --- a/hphp/runtime/base/execution_context.cpp +++ b/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) { diff --git a/hphp/runtime/base/file_repository.cpp b/hphp/runtime/base/file_repository.cpp index 1bda485eb..def44be6e 100644 --- a/hphp/runtime/base/file_repository.cpp +++ b/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; } diff --git a/hphp/runtime/base/file_repository.h b/hphp/runtime/base/file_repository.h index f5fb376ac..c252e7f44 100644 --- a/hphp/runtime/base/file_repository.h +++ b/hphp/runtime/base/file_repository.h @@ -121,7 +121,20 @@ typedef RankedCHM 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: diff --git a/hphp/runtime/base/object_data.cpp b/hphp/runtime/base/object_data.cpp index 6af8c08da..653031b91 100644 --- a/hphp/runtime/base/object_data.cpp +++ b/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. diff --git a/hphp/runtime/ext/ext_reflection.cpp b/hphp/runtime/ext/ext_reflection.cpp index d500696a0..b0e818be6 100644 --- a/hphp/runtime/ext/ext_reflection.cpp +++ b/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 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)); } diff --git a/hphp/runtime/ext/ext_spl.cpp b/hphp/runtime/ext/ext_spl.cpp index 551bc5da9..2011860b5 100644 --- a/hphp/runtime/ext/ext_spl.cpp +++ b/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; diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index d153fcdb3..e0ff3340d 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/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; } diff --git a/hphp/runtime/vm/class.cpp b/hphp/runtime/vm/class.cpp index 713b8cdaa..17a678022 100644 --- a/hphp/runtime/vm/class.cpp +++ b/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(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(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(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::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(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); } } diff --git a/hphp/runtime/vm/class.h b/hphp/runtime/vm/class.h index 16dd0cd06..24c4a76ab 100644 --- a/hphp/runtime/vm/class.h +++ b/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 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); diff --git a/hphp/runtime/vm/fixed_string_map.cpp b/hphp/runtime/vm/fixed_string_map.cpp index 9803cc07f..11ed4a9b6 100644 --- a/hphp/runtime/vm/fixed_string_map.cpp +++ b/hphp/runtime/vm/fixed_string_map.cpp @@ -38,10 +38,10 @@ inline bool strEqual(bool case_sensitive, } template -FixedStringMap::~FixedStringMap() { - if (m_table != (Elm*)&null_key) { - free(m_table); - } +void FixedStringMap::clear() { + if (m_table != (Elm*)&null_key) free(m_table); + m_table = nullptr; + m_mask = 0; } template diff --git a/hphp/runtime/vm/fixed_string_map.h b/hphp/runtime/vm/fixed_string_map.h index abecf9f43..fe3f41012 100644 --- a/hphp/runtime/vm/fixed_string_map.h +++ b/hphp/runtime/vm/fixed_string_map.h @@ -27,8 +27,8 @@ template 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; diff --git a/hphp/runtime/vm/func.cpp b/hphp/runtime/vm/func.cpp index 150832702..0066583af 100644 --- a/hphp/runtime/vm/func.cpp +++ b/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); diff --git a/hphp/runtime/vm/indexed_string_map.h b/hphp/runtime/vm/indexed_string_map.h index c1ef3ec41..870dcb772 100644 --- a/hphp/runtime/vm/indexed_string_map.h +++ b/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. diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index a71fd1f0f..c22f7c25a 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/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); } diff --git a/hphp/runtime/vm/jit/translator-x64.cpp b/hphp/runtime/vm/jit/translator-x64.cpp index 9096ef0b2..592911bca 100644 --- a/hphp/runtime/vm/jit/translator-x64.cpp +++ b/hphp/runtime/vm/jit/translator-x64.cpp @@ -4047,6 +4047,4 @@ void TranslatorX64::invalidateSrcKey(SrcKey sk) { } // HPHP::Transl -TRACE_SET_MOD(tx64); - } // HPHP diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp index a33a1aaf0..477ca33b7 100644 --- a/hphp/runtime/vm/jit/translator.cpp +++ b/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[] = { diff --git a/hphp/runtime/vm/jit/translator.h b/hphp/runtime/vm/jit/translator.h index 413bd5a0b..02168d517 100644 --- a/hphp/runtime/vm/jit/translator.h +++ b/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. diff --git a/hphp/runtime/vm/treadmill.cpp b/hphp/runtime/vm/treadmill.cpp index 0c16d647e..d300f40cf 100644 --- a/hphp/runtime/vm/treadmill.cpp +++ b/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)); } diff --git a/hphp/runtime/vm/treadmill.h b/hphp/runtime/vm/treadmill.h index 7f630b6af..49eabe728 100644 --- a/hphp/runtime/vm/treadmill.h +++ b/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()(); }; diff --git a/hphp/runtime/vm/unit.cpp b/hphp/runtime/vm/unit.cpp index 941143f05..7c79f28fa 100644 --- a/hphp/runtime/vm/unit.cpp +++ b/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(); }