#169 - Githubissues

er1z commented 8 years ago

See issue #169.

This commit fixes weird issue. To clarify what was wrong:

addFunction needs $context param to be a reference, not value. Someone (I have not checked who) ignored docs and passed the second one,
when I extracted array to a local variable, segfault also occurred - probably due to the incorrect garbage collection,
configuration (and object with worker) left in GearmanExecute::$workersBucket survives GC and works ok.

daum commented 8 years ago

Can you please provide a test case for where this fails just so we can verify/see what is occurring.

er1z commented 8 years ago

Clone: https://github.com/er1z/gearman-bundle-test

Enqueue: php bin/console app:test Worker: php bin/console gear:jo:e AppBundleWorkerGeneratorWorker~process -n

What causes:

@worker\servers :

    #localhost - 127.0.0.1:4730

@Worker\description :

    No description is defined

@job\methodName : process
@job\callableName : AppBundleWorkerGeneratorWorker~process
@job\supervisord : /usr/bin/php /workspace/appstore/gearman-test/app/console gearman:job:execute AppBundleWorkerGeneratorWorker~process --no-interaction
@job\iterations : 150
@job\defaultMethod : doNormal

@job\servers :

    localhost - 127.0.0.1:4730

@job\description :

    #No description is defined

[2016-09-20 12:28:58] loaded. Ctrl+C to break
zsh: segmentation fault (core dumped)  php bin/console gear:jo:e AppBundleWorkerGeneratorWorker~process -n

On:

 % php -v
PHP 7.0.10-2+deb.sury.org~trusty+1 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.10-2+deb.sury.org~trusty+1, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.4.1, Copyright (c) 2002-2016, by Derick Rethans

er1z commented 8 years ago

Somehow this build segfaulted on running phpunit. Dafq?

daum commented 7 years ago

Yep just tried it locally to to see, and getting the segfault. I don't have time to try to dive into this right now but will try to look in a couple of weeks.

mikaelkael commented 7 years ago

SGTM :+1: !

malarzm commented 7 years ago

Just want to confirm that with this PR applied I got rid of

[InvalidArgumentException]                                               
$context shall be an array with job_object_instance and job_method key.

that was thrown on every 2nd job when using PHP 7.1.1 and everything is working as expected, would be great to see this merged :+1:

daum commented 7 years ago

@malarzm We need to get the CI running properly. I haven't had a chance to look into this. If you can look into and get it passing the CI we can work on getting this merged in.

malarzm commented 7 years ago

@daum could you re-run travis to see if the segfault is still there?

daum commented 7 years ago

Just kicked it off.

malarzm commented 7 years ago

All right, I see the segfault is still there. I'll give it a spin locally on PHP 7.1 to see if it's there too, if not I'll try to have PHP 5.6 running

daum commented 7 years ago

Thanks we should probably add PHP 7.0 and 7.1 to the CI too since we support it. If you have a few to add those into the config so they are running too it'd be appreciated!

mikaelkael commented 7 years ago

I integrated it in my fork: https://github.com/mikaelkael/GearmanBundle ; the build is OK => https://travis-ci.org/mikaelkael/GearmanBundle

malarzm commented 7 years ago

I think I've found the reason of segfault, I'll try to have it up and running on PHP 7.1 and then submit a PR

daum commented 7 years ago

Awesome thanks @malarzm

mikaelkael commented 7 years ago

Be careful of the gearman extension version:

PHP 5.x: http://pecl.php.net/get/gearman/1.1.2
PHP 7.x: https://github.com/wcgallego/pecl-gearman/archive/gearman-2.0.1.tar.gz

malarzm commented 7 years ago

@mikaelkael will you submit update to travis configuration to run on PHP 7 as you have it ready in your fork?

daum commented 7 years ago

This was merged in via https://github.com/mmoreram/GearmanBundle/pull/175

mmoreram / GearmanBundle

#169 #172