[apc] patch to fix class inherit duplicate method opcode bug

i can't find the bug reported by myself long ago, it's just missing so i post the patch here and my_uninsulate part of the patch is from eacelerator(turck-mmcache) with GPL :( but it's so simple, almost the same as install_function()/install_class() in apc. you might rewrite one for licence issue. the other part is written by myself. the do/while/if part is a rewritten code just for better flow, with lower chance to get mistake. and i also recommend this struct to other part of source code in apc. maybe i can be better.

Index: apc_main.c =================================================================== RCS file: /repository/pecl/apc/apc_main.c,v retrieving revision 3.27 diff -u -r3.27 apc_main.c --- apc_main.c 3 Feb 2005 15:33:12 -0000 3.27 +++ apc_main.c 14 Feb 2005 16:56:39 -0000 @@ -119,6 +117,102 @@ } /* }}} */ +/* {{{ my_insulate + Overrides zend_compile_file */ +typedef struct { + HashTable* orig_function_table; + HashTable* orig_class_table; + HashTable tmp_function_table; + HashTable tmp_class_table; +} my_insulate_storage; + +static void my_insulate(my_insulate_storage *p TSRMLS_DC) +{ + zend_hash_init_ex(&(p->tmp_function_table), 100, NULL, NULL, 1, 0); + p->orig_function_table = CG(function_table); + CG(function_table) = &(p->tmp_function_table); + + zend_hash_init_ex(&(p->tmp_class_table), 10, NULL, NULL, 1, 0); + p->orig_class_table = CG(class_table); + CG(class_table) = &(p->tmp_class_table); +} + +static void my_uninsulate(my_insulate_storage *p, zend_file_handle* file_handle TSRMLS_DC) +{ + Bucket *p_function; + Bucket *p_class; + + p_function = p->tmp_function_table.pListHead; + p_class = p->tmp_class_table.pListHead; + + CG(function_table) = p->orig_function_table; + CG(class_table) = p->orig_class_table; + + /* install function */ + while (p_function != NULL) { + zend_op_array *op_array = (zend_op_array*)p_function->pData; + if (op_array->type == ZEND_USER_FUNCTION) { + if (zend_hash_add(CG(function_table), + p_function->arKey, + p_function->nKeyLength, + op_array, sizeof(zend_op_array), NULL) == FAILURE && + p_function->arKey[0] != '\000') { + CG(in_compilation) = 1; + CG(compiled_filename) = file_handle->opened_path; +#ifdef ZEND_ENGINE_2 + CG(zend_lineno) = op_array->line_start; +#else + CG(zend_lineno) = op_array->opcodes[0].lineno; +#endif + zend_error(E_ERROR, "Cannot redeclare %s()", p_function->arKey); + } + } + p_function = p_function->pListNext; + } + + /* install class */ + while (p_class != NULL) { +#ifdef ZEND_ENGINE_2 + zend_class_entry **ce = (zend_class_entry**)p_class->pData; + if ((*ce)->type == ZEND_USER_CLASS) { + if (zend_hash_add(CG(class_table), + p_class->arKey, + p_class->nKeyLength, + ce, sizeof(zend_class_entry*), NULL) == FAILURE && + p_class->arKey[0] != '\000') { + CG(in_compilation) = 1; + CG(compiled_filename) = file_handle->opened_path; + CG(zend_lineno) = (*ce)->line_start; + zend_error(E_ERROR, "Cannot redeclare class %s", p_class->arKey); + } + } +#else + zend_class_entry *ce = (zend_class_entry*)p_class->pData; + if (ce->type == ZEND_USER_CLASS) { + if (ce->parent != NULL) { + if (zend_hash_find(CG(class_table), (void*)ce->parent->name, ce->parent->name_length+1, (void **)&ce->parent) != SUCCESS) { + ce->parent = NULL; + } + } + if (zend_hash_add(CG(class_table), + p_class->arKey, + p_class->nKeyLength, + ce, sizeof(zend_class_entry), NULL) == FAILURE && + p_class->arKey[0] != '\000') { + CG(in_compilation) = 1; + CG(compiled_filename) = file_handle->opened_path; + CG(zend_lineno) = 0; + zend_error(E_ERROR, "Cannot redeclare class %s", p_class->arKey); + } + } +#endif + p_class = p_class->pListNext; + } + zend_hash_destroy(&(p->tmp_function_table)); + zend_hash_destroy(&(p->tmp_class_table)); +} +/* }}} */ + /* {{{ my_compile_file Overrides zend_compile_file */ static zend_op_array* my_compile_file(zend_file_handle* h, @@ -132,10 +226,16 @@ apc_function_t* alloc_functions; apc_class_t* alloc_classes; time_t t; + my_insulate_storage my_ins; + int bailout; + + if (!h->opened_path) { + return old_compile_file(h, type TSRMLS_CC); + } /* check our regular expression filters first */ if (APCG(compiled_filters)) { - int ret = apc_regex_match_array(APCG(compiled_filters), h->filename); + int ret = apc_regex_match_array(APCG(compiled_filters), h->opened_path); if(ret == APC_NEGATIVE_MATCH || (ret != APC_POSITIVE_MATCH && !APCG(cache_by_default))) { return old_compile_file(h, type TSRMLS_CC); } @@ -152,12 +252,12 @@ #endif /* try to create a cache key; if we fail, give up on caching */ - if (!apc_cache_make_file_key(&key, h->filename, PG(include_path), t TSRMLS_CC)) { + if (!apc_cache_make_file_key(&key, h->opened_path, PG(include_path), t TSRMLS_CC)) { return old_compile_file(h, type TSRMLS_CC); } /* search for the file in the cache */ - cache_entry = apc_cache_find(APCG(cache), key, t); + cache_entry = apc_cache_find(apc_cache, key, t); if (cache_entry != NULL) { int dummy = 1; if (h->opened_path == NULL) { @@ -166,62 +266,77 @@ zend_hash_add(&EG(included_files), h->opened_path, strlen(h->opened_path)+1, (void *)&dummy, sizeof(int), NULL); zend_llist_add_element(&CG(open_files), h); /* XXX kludge */ apc_stack_push(APCG(cache_stack), cache_entry); - return cached_compile(TSRMLS_C); + return cached_compile(cache_entry TSRMLS_CC); } + /* move to a new func/class env */ + my_insulate(&my_ins TSRMLS_CC); + /* remember how many functions and classes existed before compilation */ num_functions = zend_hash_num_elements(CG(function_table)); num_classes = zend_hash_num_elements(CG(class_table)); - /* compile the file using the default compile function */ - op_array = old_compile_file(h, type TSRMLS_CC); - if (op_array == NULL) { - return NULL; + zend_try { + bailout = 0; + /* compile the file using the default compile function */ + op_array = old_compile_file(h, type TSRMLS_CC); + if (op_array == NULL) { + return NULL; + } + } zend_catch { + bailout = 1; + } zend_end_try(); + if (bailout) { + my_uninsulate(&my_ins, h TSRMLS_CC); + zend_bailout(); } HANDLE_BLOCK_INTERRUPTIONS(); - if(!(alloc_op_array = apc_copy_op_array(NULL, op_array, apc_sma_malloc, apc_sma_free TSRMLS_CC))) { - apc_cache_expunge(APCG(cache),t); - HANDLE_UNBLOCK_INTERRUPTIONS(); - apc_log(APC_WARNING, "(apc_copy_op_array) unable to cache '%s': insufficient " "shared memory available", h->opened_path?h->opened_path:""); - return op_array; - } - - if(!(alloc_functions = apc_copy_new_functions(num_functions, apc_sma_malloc, apc_sma_free TSRMLS_CC))) { - apc_free_op_array(alloc_op_array, apc_sma_free); - apc_cache_expunge(APCG(cache),t); - HANDLE_UNBLOCK_INTERRUPTIONS(); - apc_log(APC_WARNING, "(apc_copy_new_functions) unable to cache '%s': insufficient " "shared memory available", h->opened_path?h->opened_path:""); - return op_array; - } - if(!(alloc_classes = apc_copy_new_classes(op_array, num_classes, apc_sma_malloc, apc_sma_free TSRMLS_CC))) { - apc_free_op_array(alloc_op_array, apc_sma_free); - apc_free_functions(alloc_functions, apc_sma_free); - apc_cache_expunge(APCG(cache),t); - HANDLE_UNBLOCK_INTERRUPTIONS(); - apc_log(APC_WARNING, "(apc_copy_new_classes) unable to cache '%s': insufficient " "shared memory available", h->opened_path?h->opened_path:""); - return op_array; - } - - if(!(cache_entry = apc_cache_make_file_entry(h->opened_path, alloc_op_array, alloc_functions, alloc_classes))) { - apc_free_op_array(alloc_op_array, apc_sma_free); - apc_free_functions(alloc_functions, apc_sma_free); - apc_free_classes(alloc_classes, apc_sma_free); - apc_cache_expunge(APCG(cache),t); + do { + alloc_classes = NULL; + alloc_functions = NULL; + alloc_op_array = NULL; + if(!(alloc_op_array = apc_copy_op_array(NULL, op_array, apc_sma_malloc, apc_sma_free TSRMLS_CC))) { + apc_log(APC_WARNING, "(apc_copy_op_array) unable to cache '%s': insufficient " "shared memory available", h->opened_path); + break; + } + if(!(alloc_functions = apc_copy_new_functions(num_functions, apc_sma_malloc, apc_sma_free TSRMLS_CC))) { + apc_log(APC_WARNING, "(apc_copy_new_functions) unable to cache '%s': insufficient " "shared memory available", h->opened_path); + break; + } + if(!(alloc_classes = apc_copy_new_classes(op_array, num_classes, apc_sma_malloc, apc_sma_free TSRMLS_CC))) { + apc_log(APC_WARNING, "(apc_copy_new_classes) unable to cache '%s': insufficient " "shared memory available", h->opened_path); + break; + } + if(!(cache_entry = apc_cache_make_file_entry(h->opened_path, alloc_op_array, alloc_functions, alloc_classes))) { + apc_log(APC_WARNING, "(apc_cache_make_file_entry) unable to cache '%s': insufficient " "shared memory available", h->opened_path); + break; + } + } while (0); + if (cache_entry) { HANDLE_UNBLOCK_INTERRUPTIONS(); - apc_log(APC_WARNING, "(apc_cache_make_file_entry) unable to cache '%s': insufficient " "shared memory available", h->opened_path?h->opened_path:""); - return op_array; + if ((ret = apc_cache_insert(apc_cache, key, cache_entry, t)) != 1) { + apc_cache_free_entry(cache_entry); + if (ret==-1) { + apc_cache_expunge(apc_cache,t); + apc_log(APC_WARNING, "(apc_cache_insert) unable to cache '%s': insufficient " "shared memory available", h->opened_path); + } + } } - HANDLE_UNBLOCK_INTERRUPTIONS(); - - if ((ret = apc_cache_insert(APCG(cache), key, cache_entry, t)) != 1) { - apc_cache_free_entry(cache_entry); - if(ret==-1) { - apc_cache_expunge(APCG(cache),t); - apc_log(APC_WARNING, "(apc_cache_insert) unable to cache '%s': insufficient " "shared memory available", h->opened_path?h->opened_path:""); + else { + if (alloc_classes != NULL) { + apc_free_classes(alloc_classes, apc_sma_free); + } + if (alloc_functions != NULL) { + apc_free_functions(alloc_functions, apc_sma_free); } + if (alloc_op_array != NULL) { + apc_free_op_array(alloc_op_array, apc_sma_free); + } + apc_cache_expunge(apc_cache,t); + HANDLE_UNBLOCK_INTERRUPTIONS(); } - + my_uninsulate(&my_ins, h TSRMLS_CC); return op_array; } /* }}} */