From 991ef7898acc361044c647f65072b45f91840006 Mon Sep 17 00:00:00 2001 From: Rasmus Lerdorf Date: Sat, 23 Jan 2021 22:26:29 -0800 Subject: [PATCH 1/2] Some work on PHP 8 support Still more to do because it crashes on many tests unless I run with OMP_NUM_THREADS=1 This doesn't seem to be a PHP 8 problem as I see the same with PHP 7.4, but not with 7.3 --- gmagick.c | 39 +++++++++++++++++++++++--- gmagick_methods.c | 4 --- php_gmagick.h | 51 ++++++++++++++++++++++++++++++++++ php_gmagick_macros.h | 66 ++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 146 insertions(+), 14 deletions(-) diff --git a/gmagick.c b/gmagick.c index 27ec8cc..0e9a57a 100644 --- a/gmagick.c +++ b/gmagick.c @@ -85,20 +85,37 @@ static zend_object *php_gmagick_object_new(zend_class_entry *class_type) /* {{{ static zend_object *php_gmagick_clone_gmagick_object(zval *this_ptr TSRMLS_DC) */ +#if PHP_MAJOR_VERSION < 8 static zend_object *php_gmagick_clone_gmagick_object(zval *this_ptr TSRMLS_DC) { php_gmagick_object *old_obj = Z_GMAGICK_OBJ_P(this_ptr); php_gmagick_object *new_obj = GMAGICK_FETCH_OBJECT(php_gmagick_object_new_ex(old_obj->zo.ce, 0)); - + zend_objects_clone_members(&new_obj->zo, &old_obj->zo); - + if (new_obj->magick_wand) { DestroyMagickWand(new_obj->magick_wand); } - + new_obj->magick_wand = CloneMagickWand(old_obj->magick_wand); return &new_obj->zo; } +#else +static zend_object *php_gmagick_clone_gmagick_object(zend_object *this_ptr) +{ + php_gmagick_object *old_obj = GMAGICK_FETCH_OBJECT(this_ptr); + php_gmagick_object *new_obj = GMAGICK_FETCH_OBJECT(php_gmagick_object_new_ex(this_ptr->ce, 0)); + + zend_objects_clone_members(&new_obj->zo, &old_obj->zo); + + if (new_obj->magick_wand) { + DestroyMagickWand(new_obj->magick_wand); + } + + new_obj->magick_wand = CloneMagickWand(old_obj->magick_wand); + return &new_obj->zo; +} +#endif /* }}} */ /* {{{ static void php_gmagickdraw_object_free_storage(zend_object *object) @@ -203,6 +220,7 @@ static zend_object *php_gmagickpixel_object_new(zend_class_entry *class_type TSR /* {{{ static zend_object *php_gmagick_clone_gmagickpixel_object(zval *this_ptr) */ +#if PHP_MAJOR_VERSION < 8 static zend_object *php_gmagick_clone_gmagickpixel_object(zval *this_ptr) { php_gmagickpixel_object *old_obj = Z_GMAGICKPIXEL_OBJ_P(this_ptr); @@ -213,6 +231,19 @@ static zend_object *php_gmagick_clone_gmagickpixel_object(zval *this_ptr) return &new_obj->zo; } +#else +static zend_object *php_gmagick_clone_gmagickpixel_object(zend_object *this_ptr) +{ + php_gmagickpixel_object *old_obj = GMAGICKPIXEL_FETCH_OBJECT(this_ptr); + php_gmagickpixel_object *new_obj = GMAGICKPIXEL_FETCH_OBJECT(php_gmagickpixel_object_new_ex(old_obj->zo.ce, 0)); + + zend_objects_clone_members(&new_obj->zo, &old_obj->zo); + GMAGICK_CLONE_PIXELWAND(old_obj->pixel_wand, new_obj->pixel_wand); + + return &new_obj->zo; +} + +#endif /* }}} */ ZEND_BEGIN_ARG_INFO_EX(gmagick_empty_args, 0, 0, 0) @@ -1020,7 +1051,7 @@ static zend_function_entry php_gmagick_class_methods[] = PHP_ME(gmagick, setimagewhitepoint, gmagick_setimagewhitepoint_args, ZEND_ACC_PUBLIC) PHP_ME(gmagick, setsamplingfactors, gmagick_setsamplingfactors_args, ZEND_ACC_PUBLIC) PHP_ME(gmagick, setsize, gmagick_setsize_args, ZEND_ACC_PUBLIC) - PHP_ME(gmagick, getversion, gmagick_empty_args, ZEND_ACC_PUBLIC) + PHP_ME(gmagick, getversion, gmagick_empty_args, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) PHP_ME(gmagick, getimagegeometry, gmagick_empty_args, ZEND_ACC_PUBLIC) PHP_ME(gmagick, getimage, gmagick_empty_args, ZEND_ACC_PUBLIC) PHP_ME(gmagick, setimage, gmagick_setimage_args, ZEND_ACC_PUBLIC) diff --git a/gmagick_methods.c b/gmagick_methods.c index d2a0749..39c48a4 100644 --- a/gmagick_methods.c +++ b/gmagick_methods.c @@ -3091,10 +3091,6 @@ PHP_METHOD(gmagick, getversion) char *version_string; unsigned long version_number; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "") == FAILURE) { - return; - } - version_string = (char *)MagickGetVersion(&version_number); array_init(return_value); diff --git a/php_gmagick.h b/php_gmagick.h index 91baeda..f5e25ca 100644 --- a/php_gmagick.h +++ b/php_gmagick.h @@ -57,6 +57,57 @@ typedef long ssize_t; #include "ext/standard/info.h" #include "ext/standard/php_filestat.h" +/* backward compat macros */ + +#ifndef TSRMLS_CC +#define TSRMLS_FETCH() +#define TSRMLS_CC +#define TSRMLS_DC +#define TSRMLS_D +#define TSRMLS_C +#endif + +#ifndef IS_MIXED +# define IS_MIXED 0 +#endif + +#ifndef ZEND_ARG_INFO_WITH_DEFAULT_VALUE +#define ZEND_ARG_INFO_WITH_DEFAULT_VALUE(pass_by_ref, name, default_value) \ + ZEND_ARG_INFO(pass_by_ref, name) +#endif + +#if PHP_VERSION_ID < 70200 +#undef ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX +#define ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(name, return_reference, required_num_args, class_name, allow_null) \ + static const zend_internal_arg_info name[] = { \ + { (const char*)(zend_uintptr_t)(required_num_args), ( #class_name ), 0, return_reference, allow_null, 0 }, +#endif + +#ifndef ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX +# define ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(name, return_reference, required_num_args, class_name, allow_null) \ + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(name, return_reference, required_num_args, class_name, allow_null) +#endif + +#ifndef ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX +# define ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(name, return_reference, num_args, type) \ + ZEND_BEGIN_ARG_INFO_EX(name, 0, return_reference, num_args) +#endif + +#ifndef ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX +# define ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(name, return_reference, required_num_args, class_name, type) \ + ZEND_BEGIN_ARG_INFO_EX(name, 0, return_reference, required_num_args) +#endif + +#ifndef ZEND_ARG_TYPE_MASK +# define ZEND_ARG_TYPE_MASK(pass_by_ref, name, type_mask, default_value) \ + ZEND_ARG_TYPE_INFO(pass_by_ref, name, 0, 0) +#endif + +#ifndef ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE +# define ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(pass_by_ref, name, type_hint, allow_null, default_value) \ + ZEND_ARG_TYPE_INFO(pass_by_ref, name, type_hint, allow_null) +#endif + #if GMAGICK_LIB_MASK >= 1003018 #define GMAGICK_HAVE_SET_IMAGE_PAGE 1 #endif diff --git a/php_gmagick_macros.h b/php_gmagick_macros.h index f0e204f..71da42f 100644 --- a/php_gmagick_macros.h +++ b/php_gmagick_macros.h @@ -31,7 +31,7 @@ #define GMAGICKPIXEL_FETCH_OBJECT(zv_p) (php_gmagickpixel_object *)((char*)(zv_p) - XtOffsetOf(php_gmagickpixel_object, zo)) #define Z_GMAGICKPIXEL_OBJ_P(zv) GMAGICKPIXEL_FETCH_OBJECT(Z_OBJ_P((zv))) - + /* }}} */ /* Define a set of macros to throw exceptions */ @@ -232,6 +232,7 @@ #endif /* {{{ GMAGICK_CLONE_PIXELWAND(source, target) */ +#if PHP_MAJOR_VERSION < 8 #define GMAGICK_CAST_PARAMETER_TO_COLOR(param, internp, caller) \ switch (Z_TYPE_P(param)) { \ case IS_STRING: \ @@ -257,7 +258,35 @@ default: \ GMAGICK_THROW_EXCEPTION_WITH_MESSAGE(caller, "Invalid parameter provided", (long)caller); \ break; \ - } \ + } +#else +#define GMAGICK_CAST_PARAMETER_TO_COLOR(param, internp, caller) \ + switch (Z_TYPE_P(param)) { \ + case IS_STRING: \ + { \ + zval object; \ + PixelWand *pixel_wand = NewPixelWand(); \ + if (!PixelSetColor(pixel_wand, Z_STRVAL_P(param))) { \ + GMAGICK_THROW_GMAGICKPIXEL_EXCEPTION(pixel_wand, "Unrecognized color string"); \ + return; \ + } \ + object_init_ex(&object, php_gmagickpixel_sc_entry); \ + internp = Z_GMAGICKPIXEL_OBJ_P(&object); \ + GMAGICKPIXEL_REPLACE_PIXELWAND(internp, pixel_wand); \ + } \ + break; \ + case IS_OBJECT: \ + if (Z_OBJCE_P(param) == php_gmagickpixel_sc_entry) { \ + internp = Z_GMAGICKPIXEL_OBJ_P(param); \ + } else { \ + GMAGICK_THROW_EXCEPTION_WITH_MESSAGE(caller, "The parameter must be an instance of GmagickPixel or a string", (long)caller); \ + } \ + break; \ + default: \ + GMAGICK_THROW_EXCEPTION_WITH_MESSAGE(caller, "Invalid parameter provided", (long)caller); \ + break; \ + } +#endif /* }}} */ /* {{{ GMAGICK_CLONE_PIXELWAND(source, target) */ @@ -316,9 +345,9 @@ }\ RETURN_NULL();\ break;\ - } \ -/* }}} */ + } +#if PHP_MAJOR_VERSION < 7 #define GMAGICK_CAST_PARAMETER_TO_OPACITY(param, internp, caller) \ switch (Z_TYPE_P(param)) { \ case IS_LONG: \ @@ -342,8 +371,33 @@ default: \ GMAGICK_THROW_EXCEPTION_WITH_MESSAGE(caller, "Invalid parameter provided", (long)caller); \ break; \ - } \ -/* }}} */ + } +#else +#define GMAGICK_CAST_PARAMETER_TO_OPACITY(param, internp, caller) \ + switch (Z_TYPE_P(param)) { \ + case IS_LONG: \ + case IS_DOUBLE: \ + { \ + zval object; \ + PixelWand *pixel_wand = NewPixelWand(); \ + PixelSetOpacity(pixel_wand, Z_DVAL_P(param)); \ + object_init_ex(&object, php_gmagickpixel_sc_entry); \ + internp = Z_GMAGICKPIXEL_OBJ_P(&object); \ + GMAGICKPIXEL_REPLACE_PIXELWAND(internp, pixel_wand); \ + } \ + break; \ + case IS_OBJECT: \ + if (Z_OBJCE_P(param) == php_gmagickpixel_sc_entry) { \ + internp = Z_GMAGICKPIXEL_OBJ_P(param); \ + } else { \ + GMAGICK_THROW_EXCEPTION_WITH_MESSAGE(caller, "The parameter must be an instance of GmagickPixel or a string", (long)caller); \ + } \ + break; \ + default: \ + GMAGICK_THROW_EXCEPTION_WITH_MESSAGE(caller, "Invalid parameter provided", (long)caller); \ + break; \ + } +#endif /* {{{ GMAGICK_SAFEMODE_OPENBASEDIR_CHECK(filename) */ #if PHP_VERSION_ID > 50399 From 21176bc6be8639198ab5fe4ed5925f42ead2a156 Mon Sep 17 00:00:00 2001 From: Rasmus Lerdorf Date: Sun, 24 Jan 2021 21:44:28 -0800 Subject: [PATCH 2/2] Add set_single_thread and shutdown_sleep_count ini settings to match the imagick extension but also add an config check for omp_pause_resource_all() and use that if available --- config.m4 | 2 ++ gmagick.c | 38 +++++++++++++++++++++++++++++++++++++- php_gmagick.h | 19 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/config.m4 b/config.m4 index c7d7c9f..8bd545a 100644 --- a/config.m4 +++ b/config.m4 @@ -30,6 +30,8 @@ if test $PHP_GMAGICK != "no"; then AC_MSG_CHECKING(GraphicsMagick version mask) AC_MSG_RESULT(found version $GRAPHICSMAGICK_VERSION_MASK) + PHP_CHECK_FUNC(omp_pause_resource_all, gomp) + LIB_DIR=$WAND_DIR/lib # If "$LIB_DIR" == "/usr/lib" or possible /usr/$PHP_LIBDIR" then you're probably # going to have a bad time. PHP m4 files seem to be hard-coded to not link properly against diff --git a/gmagick.c b/gmagick.c index 0e9a57a..67256ec 100644 --- a/gmagick.c +++ b/gmagick.c @@ -21,6 +21,8 @@ #include "php_gmagick_macros.h" #include "php_gmagick_helpers.h" +ZEND_DECLARE_MODULE_GLOBALS(gmagick) + /* handlers */ static zend_object_handlers gmagick_object_handlers; static zend_object_handlers gmagickdraw_object_handlers; @@ -1690,6 +1692,18 @@ static zend_function_entry php_gmagickpixel_class_methods[] = }; /* }}} */ +PHP_INI_BEGIN() + STD_PHP_INI_ENTRY("gmagick.set_single_thread", "0", PHP_INI_SYSTEM, OnUpdateBool, set_single_thread, zend_gmagick_globals, gmagick_globals) + STD_PHP_INI_ENTRY("gmagick.shutdown_sleep_count", "10", PHP_INI_ALL, OnUpdateLong, shutdown_sleep_count, zend_gmagick_globals, gmagick_globals) +PHP_INI_END() + +static void php_gmagick_init_globals(zend_gmagick_globals *gmagick_globals) +{ + gmagick_globals->set_single_thread = 0; + // 10 is magick number, that seems to be enough. + gmagick_globals->shutdown_sleep_count = 10; +} + /* {{{ PHP_MINIT_FUNCTION(gmagick) */ PHP_MINIT_FUNCTION(gmagick) @@ -1698,6 +1712,8 @@ PHP_MINIT_FUNCTION(gmagick) size_t cwd_len; zend_class_entry ce; + + ZEND_INIT_MODULE_GLOBALS(gmagick, php_gmagick_init_globals, NULL); /* Exception */ INIT_CLASS_ENTRY(ce, "GmagickException", NULL); @@ -1738,12 +1754,20 @@ PHP_MINIT_FUNCTION(gmagick) if (!cwd) return FAILURE; - + + InitializeMagick(cwd); efree(cwd); /* init constants */ php_gmagick_initialize_constants(); + + REGISTER_INI_ENTRIES(); + + if (GMAGICK_G(set_single_thread)) { + MagickSetResourceLimit(ThreadsResource, 1); + } + return SUCCESS; } /* }}} */ @@ -1753,6 +1777,16 @@ PHP_MINIT_FUNCTION(gmagick) PHP_MSHUTDOWN_FUNCTION(gmagick) { DestroyMagick(); +#if HAVE_OMP_PAUSE_RESOURCE_ALL + // Note there is a patch to add omp_pause_resource_all to DestroyMagick() + // https://sourceforge.net/p/graphicsmagick/patches/63/ + // But it hasn't been accepted + omp_pause_resource_all(omp_pause_hard); +#else + for (i = 0; i < 100 && i < GMAGICK_G(shutdown_sleep_count); i += 1) { + usleep(1000); + } +#endif return SUCCESS; } /* }}} */ @@ -1771,6 +1805,8 @@ PHP_MINFO_FUNCTION(gmagick) php_info_print_table_row(2, "gmagick version", PHP_GMAGICK_VERSION); php_info_print_table_row(2, "GraphicsMagick version", version); php_info_print_table_end(); + + DISPLAY_INI_ENTRIES(); } /* {{{ zend_module_entry gmagick_module_entry diff --git a/php_gmagick.h b/php_gmagick.h index f5e25ca..25ddc56 100644 --- a/php_gmagick.h +++ b/php_gmagick.h @@ -29,6 +29,7 @@ /* Include GraphicsMagick header */ #include #include + #if defined(PHP_WIN32) && !defined(ssize_t) /* XXX actually wand_api.h should be included after php.h, ssize_t were there with much more probability. */ @@ -57,6 +58,10 @@ typedef long ssize_t; #include "ext/standard/info.h" #include "ext/standard/php_filestat.h" +#if HAVE_OMP_PAUSE_RESOURCE_ALL +#include +#endif + /* backward compat macros */ #ifndef TSRMLS_CC @@ -134,6 +139,20 @@ typedef struct _php_gmagickpixel_object { zend_object zo; } php_gmagickpixel_object; +/* Globals, needed for the ini settings */ +ZEND_BEGIN_MODULE_GLOBALS(gmagick) + zend_bool set_single_thread; + zend_long shutdown_sleep_count; +ZEND_END_MODULE_GLOBALS(gmagick) + +#ifdef ZTS +# define GMAGICK_G(v) TSRMG(gmagick_globals_id, zend_gmagick_globals *, v) +#else +# define GMAGICK_G(v) (gmagick_globals.v) +#endif + +ZEND_EXTERN_MODULE_GLOBALS(gmagick) + extern zend_module_entry gmagick_module_entry; #define phpext_gmagick_ptr &gmagick_module_entry