Skip to content

Commit

Permalink
Don't intern compiled_filename
Browse files Browse the repository at this point in the history
For php-ast interning the file name is an effective memory leak,
see php-ast#134.

I don't think there's any reason to do this. At some point this
was needed due to bugs in the interned string mechanism that
caused issues if the string was later interned, e.g. through a
__FILE__ reference. These issues have since been resolved.

In conjunction with the filenames_table removal in c4016ec
this means that filenames now need to be refcounted like normal
strings. In particular the filename reference in op_arrays and CEs
are refcounted.
  • Loading branch information
nikic committed Sep 3, 2020
1 parent c4016ec commit 7620ea1
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 15 deletions.
9 changes: 6 additions & 3 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,14 +442,17 @@ void shutdown_compiler(void) /* {{{ */

ZEND_API zend_string *zend_set_compiled_filename(zend_string *new_compiled_filename) /* {{{ */
{
new_compiled_filename = zend_new_interned_string(zend_string_copy(new_compiled_filename));
CG(compiled_filename) = new_compiled_filename;
CG(compiled_filename) = zend_string_copy(new_compiled_filename);
return new_compiled_filename;
}
/* }}} */

ZEND_API void zend_restore_compiled_filename(zend_string *original_compiled_filename) /* {{{ */
{
if (CG(compiled_filename)) {
zend_string_release(CG(compiled_filename));
CG(compiled_filename) = NULL;
}
CG(compiled_filename) = original_compiled_filename;
}
/* }}} */
Expand Down Expand Up @@ -7345,7 +7348,7 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) /
}

ce->ce_flags |= decl->flags;
ce->info.user.filename = zend_get_compiled_filename();
ce->info.user.filename = zend_string_copy(zend_get_compiled_filename());
ce->info.user.line_start = decl->start_lineno;
ce->info.user.line_end = decl->end_lineno;

Expand Down
3 changes: 2 additions & 1 deletion Zend/zend_language_scanner.l
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,9 @@ ZEND_API void zend_save_lexical_state(zend_lex_state *lex_state)

lex_state->in = SCNG(yy_in);
lex_state->yy_state = YYSTATE;
lex_state->filename = zend_get_compiled_filename();
lex_state->filename = CG(compiled_filename);
lex_state->lineno = CG(zend_lineno);
CG(compiled_filename) = NULL;

lex_state->script_org = SCNG(script_org);
lex_state->script_org_size = SCNG(script_org_size);
Expand Down
4 changes: 3 additions & 1 deletion Zend/zend_opcode.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz
op_array->T = 0;

op_array->function_name = NULL;
op_array->filename = zend_get_compiled_filename();
op_array->filename = zend_string_copy(zend_get_compiled_filename());
op_array->doc_comment = NULL;
op_array->attributes = NULL;

Expand Down Expand Up @@ -354,6 +354,7 @@ ZEND_API void destroy_zend_class(zval *zv)
}
efree(ce->interfaces);
}
zend_string_release_ex(ce->info.user.filename, 0);
if (ce->info.user.doc_comment) {
zend_string_release_ex(ce->info.user.doc_comment, 0);
}
Expand Down Expand Up @@ -496,6 +497,7 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
}
efree(op_array->opcodes);

zend_string_release_ex(op_array->filename, 0);
if (op_array->doc_comment) {
zend_string_release_ex(op_array->doc_comment, 0);
}
Expand Down
9 changes: 3 additions & 6 deletions ext/opcache/ZendAccelerator.c
Original file line number Diff line number Diff line change
Expand Up @@ -4528,7 +4528,6 @@ static int accel_preload(const char *config, zend_bool in_child)

if (ret == SUCCESS) {
zend_persistent_script *script;
zend_string *filename;
int ping_auto_globals_mask;
int i;
zend_class_entry *ce;
Expand Down Expand Up @@ -4668,7 +4667,7 @@ static int accel_preload(const char *config, zend_bool in_child)
script->ping_auto_globals_mask = ping_auto_globals_mask;

/* Store all functions and classes in a single pseudo-file */
filename = zend_string_init("$PRELOAD$", sizeof("$PRELOAD$") - 1, 0);
CG(compiled_filename) = zend_string_init("$PRELOAD$", sizeof("$PRELOAD$") - 1, 0);
#if ZEND_USE_ABS_CONST_ADDR
init_op_array(&script->script.main_op_array, ZEND_USER_FUNCTION, 1);
#else
Expand All @@ -4690,8 +4689,8 @@ static int accel_preload(const char *config, zend_bool in_child)
ZEND_PASS_TWO_UPDATE_CONSTANT(&script->script.main_op_array, script->script.main_op_array.opcodes, script->script.main_op_array.opcodes[0].op1);
zend_vm_set_opcode_handler(script->script.main_op_array.opcodes);

script->script.main_op_array.filename = filename;
script->script.filename = zend_string_copy(filename);
script->script.filename = CG(compiled_filename);
CG(compiled_filename) = NULL;

script->script.first_early_binding_opline = (uint32_t)-1;

Expand Down Expand Up @@ -4728,8 +4727,6 @@ static int accel_preload(const char *config, zend_bool in_child)
SHM_PROTECT();
HANDLE_UNBLOCK_INTERRUPTIONS();

zend_string_release(filename);

ZEND_ASSERT(ZCSG(preload_script)->arena_size == 0);

preload_load();
Expand Down
6 changes: 2 additions & 4 deletions ext/opcache/zend_persist.c
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,7 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc
}

if (op_array->filename) {
/* do not free! PHP has centralized filename storage, compiler will free it */
zend_accel_memdup_string(op_array->filename);
zend_accel_store_string(op_array->filename);
}

if (op_array->arg_info) {
Expand Down Expand Up @@ -864,8 +863,7 @@ static void zend_persist_class_entry(zval *zv)
HT_FLAGS(&ce->constants_table) &= (HASH_FLAG_UNINITIALIZED | HASH_FLAG_STATIC_KEYS);

if (ce->info.user.filename) {
/* do not free! PHP has centralized filename storage, compiler will free it */
zend_accel_memdup_string(ce->info.user.filename);
zend_accel_store_string(ce->info.user.filename);
}
if (ce->info.user.doc_comment) {
if (ZCG(accel_directives).save_comments) {
Expand Down

0 comments on commit 7620ea1

Please sign in to comment.