php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #59706 Uncaught worker exception sends back GEARMAN_SUCCESS return code.
Submitted: 2011-04-10 15:24 UTC Modified: 2011-08-20 07:41 UTC
From: hradtke@php.net Assigned: hradtke (profile)
Status: Closed Package: gearman (PECL)
PHP Version: Trunk SVN-2011-04-10 (dev) OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: hradtke@php.net
New email:
PHP Version: OS:

 

 [2011-04-10 15:24 UTC] hradtke@php.net 
Description:
------------
The gearman worker code return GEARMAN_SUCCESS if an uncaught exception is thrown in worker callback function.

Reproduce code:
---------------
<?php
$gmworker= new GearmanWorker();
$gmworker->addServer();
$gmworker->addFunction("func", "func");

while($gmworker->work());

function func($job)
{
    throw new Exception('oww!');
}
?>

Patch:
svn diff php_gearman.c
Index: php_gearman.c
===================================================================
--- php_gearman.c       (revision 310107)
+++ php_gearman.c       (working copy)
@@ -3438,8 +3438,13 @@
                                                 worker_cb->zcall->value.str.val : "[undefined]");
                *ret_ptr= GEARMAN_WORK_FAIL;
        }
-       *ret_ptr= jobj->ret;

+       if (EG(exception)) {
+               *ret_ptr= GEARMAN_WORK_FAIL;
+       } else {
+               *ret_ptr= jobj->ret;
+       }
+
        if (zret_ptr == NULL || Z_TYPE_P(zret_ptr) == IS_NULL) {
                result= NULL;
        } else {


Expected result:
----------------
The GearmanClient::returnCode() method should return GEARMAN_WORK_FAIL.

Actual result:
--------------
The GearmanClient::returnCode() method returns GEARMAN_SUCCESS.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-04-20 07:36 UTC] craig dot lumley at everythingeverywhere dot com 
If the aim of this patch is to catch exceptions properly shouldn't it be making use of the GEARMAN_WORK_EXCEPTION constant?
 [2011-04-21 01:44 UTC] hradtke@php.net 
From what I have read of the Gearman protocol, it really does not deal with instances where a worker crashes.  An uncaught exception will cause the worker to terminate (crash) and I see a crash as a GEARMAN_WORK_FAIL.

My goal was not to involve the gearman extension in exception handling, but instead to inform the server and client that the result is not GEARMAN_SUCCESS.

I see GEARMAN_WORK_EXCEPTION as a tool for the worker to send a status up if certain exceptions are explicitly caught.
 [2011-04-21 06:13 UTC] craig dot lumley at everythingeverywhere dot com 
I see where you're coming from and further to the post on your personal site I think we should be going down the route of correctly allowing the exceptions to bubble back up to the GearmanClient and dealing with that there. It allows much more control.

I think if no exception call back (GearmanClient::setExceptionCallback) has been set we should look at returning a fail as you have suggested. Currently as you know the exception callback isn't working as suggested by the documentation.

In the instance of what information to return back, if possible a serialized version of the exception along with the job id and/or any other information that might be useful would be the most appropriate in my opinion. What're your thoughts?

Hopefully this is the right bug report to be considering these options?

- Craig
 [2011-05-15 05:41 UTC] hradtke@php.net 
Trying to get a straight answer from the libgearman guys as to whether or not GEARMAN_WORK_EXCEPTION is even supported.  If I send that status back, the client just infinite loops.
 [2011-08-20 07:41 UTC] hradtke@php.net 
I have fixed this in svn.



Due to the way libgearman currently works, GEARMAN_WORK_FAIL is still sent to the client even if GEARMAN_WORK_EXCEPTION has already been sent.

This means two things:
1. GearmanClient::do() will still return GEARMAN_WORK_FAIL on an exception.
2. The fail callback will be triggered unless the exception callback sends back a return value that is not GEARMAN_SUCCESS.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sat Dec 21 11:01:30 2024 UTC