fixed memory leak in GearmanClient::doXXX methods

[zacek]

There is a memory leak in the latest client when non-empty string (result) is returned from the worker.

See examples in gearmand - reverse_client.cc:

result= (char *)gearman_client_do(&client, "reverse", NULL,
                                      text_to_echo.c_str(), text_to_echo.size(),
                                      &result_size, &ret);
...
free(result)

The result must be freed. My fix only copies the result to the return value and frees the allocated memory.

my environment: CentOS8 with libgearman.x86_64 1.1.19.1-1.el8 @epel
php-pecl-gearman.x86_64 2.1.0-1.el8.remi.7.3 @remi-modular

[esabol]

This makes complete sense.

I note that RETURN_STRINGL is used in two other locations in the code. I wonder if similar fixes are needed there also. What do you think, @zacek?

https://github.com/php/pecl-networking-gearman/blob/69d6b78374fc9914906ecaea0fe919b6903cd526/php_gearman_task.c#L296

https://github.com/php/pecl-networking-gearman/blob/69d6b78374fc9914906ecaea0fe919b6903cd526/php_gearman_job.c#L319

[zacek]

I'd say that in these 2 cases the code is correct - it only returns reference to the data. From the documentation of these 2 methods:

gearman_task_data

gearman_task_free() is used to free a task. This only needs to be done if a task was created with a preallocated structure or if you want to clean up the memory of a specific task.

gearman_job_workload

gearman_job_free() is used to free a job. This only needs to be done if a task was created with a preallocated structure.

We only need the data, we do not want to destroy the task or job objects.

On the other hand, for the gearman_client_do function the documentation says:

gearman_client_do() returns a pointer to a value that the caller must release. If ret_ptr is provided any errors that have occurred will be stored in it. Since a NULL/zero value is a valid value, you will always need to check ret_ptr if you are concerned with errors.

[zacek]

BTW, this leak was found by monitoring a worker where the memory usage was permanently growing. After this fix the memory usage was constant. I did not see any other leaks in my code.

[esabol]

Excellent! Thanks for the explanation, @zacek!

@rlerdorf : Can you merge this?