Skip to content

Commit

Permalink
Remove partially_inited flag
Browse files Browse the repository at this point in the history
Summary:
This was added to prevent GC crashes when scanning uninitialized memory
in partially-constructed APC objects, when PHP callbacks (sprop init)
occurred during apc_fetch.

Since then, we made the GC scanner robust against uninitialized
pointers; TypedValue scanner will do nothing for bogus data types,
and plain pointers are all filtered through PtrMap, just like with
conservative scanning.

Reviewed By: ricklavoie

Differential Revision: D4891114

fbshipit-source-id: b6cb0feb4e8b68ac6e2608a49f7e879e14d0ef84
  • Loading branch information
edwinsmith authored and hhvm-bot committed Apr 15, 2017
1 parent be2968c commit f32c85f
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 15 deletions.
6 changes: 3 additions & 3 deletions hphp/runtime/base/apc-object.cpp
Expand Up @@ -237,18 +237,18 @@ Object APCObject::createObject() const {
auto const apcProp = persistentProps();

if (m_fast_init) {
obj->setPartiallyInited(true);
// re-entry is possible while we're executing toLocal() on each
// property, so heap inspectors may see partially initid objects
// not yet exposed to PHP.
unsigned i = 0;
try {
for (; i < numProps; ++i) {
new (objProp + i) Variant(apcProp[i]->toLocal());
}
obj->setPartiallyInited(false);
} catch (...) {
for (; i < numProps; ++i) {
new (objProp + i) Variant();
}
obj->setPartiallyInited(false);
throw;
}
} else {
Expand Down
3 changes: 1 addition & 2 deletions hphp/runtime/base/header-kind.h
Expand Up @@ -91,8 +91,7 @@ struct HeapObject {
mutable RefCount m_count;
HeaderKind m_kind;
mutable uint8_t m_weak_refed:1;
mutable uint8_t m_partially_inited:1;
mutable uint8_t m_marks:6;
mutable uint8_t m_marks:7;
mutable uint16_t m_aux16;
};
struct {
Expand Down
7 changes: 1 addition & 6 deletions hphp/runtime/base/heap-scan.h
Expand Up @@ -188,12 +188,7 @@ inline void c_WaitHandle::scan(type_scan::Scanner& scanner) const {

inline void ObjectData::scan(type_scan::Scanner& scanner) const {
auto props = propVec();
if (m_partially_inited) {
// we don't know which properties are initialized yet
scanner.conservative(props, m_cls->numDeclProperties() * sizeof(*props));
} else {
scanner.scan(*props, m_cls->numDeclProperties() * sizeof(*props));
}
scanner.scan(*props, m_cls->numDeclProperties() * sizeof(*props));
if (getAttribute(HasDynPropArr)) {
// nb: dynamic property arrays are in ExecutionContext::dynPropTable,
// which is not marked as a root. Scan the entry pair, so both the key
Expand Down
4 changes: 0 additions & 4 deletions hphp/runtime/base/object-data.h
Expand Up @@ -147,10 +147,6 @@ struct ObjectData : Countable, type_scan::MarkCountable<ObjectData> {
m_weak_refed = flag;
}

inline void setPartiallyInited(bool f) const {
m_partially_inited = f;
}

public:

/*
Expand Down

0 comments on commit f32c85f

Please sign in to comment.