Skip to content

Commit

Permalink
Fix zero ref-count Zend objects
Browse files Browse the repository at this point in the history 
Summary:
The Zend compat layer intentionally sets the ref-count to zero in a few
places because it attempts to do memory management manually. Since reachable
objects should never have a ref-count of zero, fix this by using the proper
ref-count primitives.

Reviewed By: bnell

Differential Revision: D2755715

fb-gh-sync-id: f0eff76a48a1f0e43381268367e1de62f736b145
  • Loading branch information
@ricklavoie @hhvm-bot
ricklavoie authored and hhvm-bot committed Dec 14, 2015
1 parent 9b9df37 commit 4eac8cb
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 30 deletions.
16 changes: 1 addition & 15 deletions hphp/runtime/base/heap-collect.cpp
View file
Expand Up @@ -367,10 +367,6 @@ Marker::operator()(const void* start, size_t len) {
}

// initially parse the heap to find valid objects and initialize metadata.
// Certain objects can have count==0
// * StringData owned by StringBuffer
// * ArrayData owned by ArrayInit
// * Object ctors allocating memory in ctor (while count still==0).
void Marker::init() {
rds_ = folly::Range<const char*>((char*)rds::header(),
RuntimeOption::EvalJitTargetCacheSize);
Expand All @@ -385,19 +381,9 @@ void Marker::init() {
case HK::Struct:
case HK::Empty:
case HK::String:
assert(h->hdr_.count > 0);
ptrs_.insert(h);
total_ += h->size();
break;
case HK::Ref:
// EZC non-ref refdatas sometimes have count==0
assert(h->hdr_.count > 0 || !h->ref_.zIsRef());
ptrs_.insert(h);
total_ += h->size();
break;
case HK::Resource:
// ZendNormalResourceData objects sometimes never incref'd
// TODO: t5969922, t6545412 might be a real bug.
assert(h->hdr_.count > 0);
ptrs_.insert(h);
total_ += h->size();
break;
Expand Down
5 changes: 2 additions & 3 deletions hphp/runtime/ext_zend_compat/mongo/mongoclient.cpp
View file
Expand Up @@ -147,7 +147,6 @@ zval *mongo_read_property(zval *object, zval *member, int type TSRMLS_DC)
char *error_message = NULL;
mongo_connection *conn = mongo_get_read_write_connection(obj->manager, obj->servers, MONGO_CON_FLAG_READ|MONGO_CON_FLAG_DONT_CONNECT, (char**) &error_message);
ALLOC_INIT_ZVAL(retval);
Z_SET_REFCOUNT_P(retval, 0);
ZVAL_BOOL(retval, conn ? 1 : 0);
if (error_message) {
free(error_message);
Expand Down Expand Up @@ -367,7 +366,7 @@ void php_mongo_ctor(INTERNAL_FUNCTION_PARAMETERS, int bc)

/* Set the manager from the global manager */
link->manager = MonGlo(manager);

/* Parse the server specification
* Default to the mongo.default_host & mongo.default_port INI options */
link->servers = mongo_parse_init();
Expand Down Expand Up @@ -646,7 +645,7 @@ PHP_METHOD(MongoClient, selectDB)
zval *new_link;
mongoclient *tmp_link;
int i;

if (strcmp(link->servers->server[0]->db, "admin") == 0) {
mongo_manager_log(
link->manager, MLOG_CON, MLOG_FINE,
Expand Down
36 changes: 24 additions & 12 deletions hphp/runtime/ext_zend_compat/php-src/Zend/zend_API.cpp
View file
Expand Up @@ -34,6 +34,18 @@
#include "hphp/runtime/vm/jit/translator-inline.h"
#include "hphp/runtime/vm/native.h"

namespace {

void releaseZval(zval* v) {
if (v->zRefcount() > 1) {
v->zDelRef();
} else {
FREE_ZVAL(v);
}
}

}

ZEND_API const char *zend_get_type_by_const(int type) {
return HPHP::getDataTypeString((HPHP::DataType)type).data();
}
Expand Down Expand Up @@ -1253,8 +1265,8 @@ ZEND_API void zend_update_property_null(zend_class_entry *scope, zval *object, c

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_NULL(tmp);
SCOPE_EXIT { releaseZval(tmp); };
zend_update_property(scope, object, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1265,8 +1277,8 @@ ZEND_API void zend_update_property_bool(zend_class_entry *scope, zval *object, c

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_BOOL(tmp, value);
SCOPE_EXIT { releaseZval(tmp); };
zend_update_property(scope, object, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1277,8 +1289,8 @@ ZEND_API void zend_update_property_long(zend_class_entry *scope, zval *object, c

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_LONG(tmp, value);
SCOPE_EXIT { releaseZval(tmp); };
zend_update_property(scope, object, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1289,8 +1301,8 @@ ZEND_API void zend_update_property_double(zend_class_entry *scope, zval *object,

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_DOUBLE(tmp, value);
SCOPE_EXIT { releaseZval(tmp); };
zend_update_property(scope, object, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1301,8 +1313,8 @@ ZEND_API void zend_update_property_string(zend_class_entry *scope, zval *object,

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_STRING(tmp, value, 1);
SCOPE_EXIT { releaseZval(tmp); };
zend_update_property(scope, object, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1313,8 +1325,8 @@ ZEND_API void zend_update_property_stringl(zend_class_entry *scope, zval *object

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_STRINGL(tmp, value, value_len, 1);
SCOPE_EXIT { releaseZval(tmp); };
zend_update_property(scope, object, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand Down Expand Up @@ -1495,8 +1507,8 @@ ZEND_API int zend_update_static_property_null(zend_class_entry *scope, const cha

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_NULL(tmp);
SCOPE_EXIT { releaseZval(tmp); };
return zend_update_static_property(scope, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1507,8 +1519,8 @@ ZEND_API int zend_update_static_property_bool(zend_class_entry *scope, const cha

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_BOOL(tmp, value);
SCOPE_EXIT { releaseZval(tmp); };
return zend_update_static_property(scope, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1519,8 +1531,8 @@ ZEND_API int zend_update_static_property_long(zend_class_entry *scope, const cha

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_LONG(tmp, value);
SCOPE_EXIT { releaseZval(tmp); };
return zend_update_static_property(scope, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1531,8 +1543,8 @@ ZEND_API int zend_update_static_property_double(zend_class_entry *scope, const c

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_DOUBLE(tmp, value);
SCOPE_EXIT { releaseZval(tmp); };
return zend_update_static_property(scope, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1543,8 +1555,8 @@ ZEND_API int zend_update_static_property_string(zend_class_entry *scope, const c

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_STRING(tmp, value, 1);
SCOPE_EXIT { releaseZval(tmp); };
return zend_update_static_property(scope, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand All @@ -1555,8 +1567,8 @@ ZEND_API int zend_update_static_property_stringl(zend_class_entry *scope, const

ALLOC_ZVAL(tmp);
Z_UNSET_ISREF_P(tmp);
Z_SET_REFCOUNT_P(tmp, 0);
ZVAL_STRINGL(tmp, value, value_len, 1);
SCOPE_EXIT { releaseZval(tmp); };
return zend_update_static_property(scope, name, name_length, tmp TSRMLS_CC);
}
/* }}} */
Expand Down

0 comments on commit 4eac8cb

Please sign in to comment.