pecl-networking-gearman icon indicating copy to clipboard operation
pecl-networking-gearman copied to clipboard

What about publishing a new version on pecl?

Open mlocati opened this issue 3 years ago • 9 comments

The master branch contains the fix https://github.com/php/pecl-networking-gearman/commit/7da13e4babc17067b2b45d6b37041c3c8ed91637

Without that patch, we can't use this extension on PHP 8.1, since we have this issue:

Warning: PHP Startup: Unable to load dynamic library 'gearman' [...]
Error relocating [...]/gearman.so: ZVAL_NEW_ARR: symbol not found

mlocati avatar Jan 11 '22 10:01 mlocati

@rlerdorf could you release new version with php 8.1 support?

oleg-st avatar Jan 12 '22 08:01 oleg-st

A new version would be highly appreciated.

Until a new release has been made, you could always build based on the master branch :)

mikevrind avatar May 09 '22 09:05 mikevrind

+1 for a new release. As php 7.4 will no longer be supported in a few months a php 8.0 supported version is needed. If there are no plans to support this the project schould be marked as abandoned.

MyDigitalLife avatar Aug 09 '22 12:08 MyDigitalLife

Who decides when (or where) to release? I'm not really clear on who has maintainer status on this project. It's under the PHP organization's GitHub?

esabol avatar Aug 09 '22 17:08 esabol

I can confirm the 'master' branch currently passes 'make test' and installs against libgearman v1.1.19 and PHP 8.1.2 (the default packages in Ubuntu 22.04). Perhaps 'master' could now be tagged a release candidate for 2.1.1? There will be many other Ubuntu 22.04 users facing the same issue with 2.1.0.

trenshaw avatar Sep 21 '22 00:09 trenshaw

+1: another reason for the new release: there is a memory leak fix in the master branch.

zacek avatar Oct 03 '22 10:10 zacek

@zacek wrote:

there is a memory leak fix in the master branch.

Could issue #17 be related to that?

esabol avatar Oct 04 '22 05:10 esabol

@esabol I don't think so. My fix was memory leak in the client. The issue in #17 is in the worker. From the report it is not clear what the worker was doing but generally, worker creates response object (allocating memory) and client is responsible for freeing it. So, my fix was correct, IMHO.

What I find very suspicious in the worker code which could cause the problem reported in #17 are these lines in the worker constructor:

	worker = Z_GEARMAN_WORKER_P(return_value);

	if (gearman_worker_create(&(worker->worker)) == NULL) {
		zval_dtor(return_value);
		GEARMAN_EXCEPTION("Memory allocation failure", 0);
	}

	worker->flags |= GEARMAN_WORKER_OBJ_CREATED;

Why is zval_dtor called for the return_value if the allocation fails? If you look into the client constructor, which does almost the same thing, the destructor for the return_value is not called at all.

IMHO, the scenario for #17 is: memory allocation for the worker fails and destructor is called on null. This also hides the real exception - insufficient memory - which would be thrown afterwards. This could easily be checked in two ways.

  1. the easy way: increase the memory and check if it mitigates the problem
  2. the more difficult way: rebuild the gearman pecl extension without the zval_dtor line in the worker constructor and try again. If you get memory allocation problem it is this problem and the destructor line should be removed

zacek avatar Oct 17 '22 06:10 zacek

Thanks for taking a look at that, @zacek. Unfortunately, my workers aren't implemented in PHP. I hope someone can test your prospective changes.

esabol avatar Oct 22 '22 14:10 esabol