From 4e26483d511576bc2e22059fe24912673e4c9cc2 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Mon, 30 Mar 2026 20:33:25 -0400 Subject: [PATCH 01/18] Add test to check that packets are FIFO. --- tests/libgearman-1.0/fifo.cc | 167 ++++++++++++++++++++++++++++++++ tests/libgearman-1.0/include.am | 22 ++++- 2 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 tests/libgearman-1.0/fifo.cc diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc new file mode 100644 index 00000000..28c796d1 --- /dev/null +++ b/tests/libgearman-1.0/fifo.cc @@ -0,0 +1,167 @@ +/* vim:expandtab:shiftwidth=2:tabstop=2:smarttab: + * + * Test that libgearman packets are sent to server in FIFO order. + * + * Copyright (C) 2026 Edward J. Sabol + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * The names of its contributors may not be used to endorse or + * promote products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "gear_config.h" + +#include + +using namespace libtest; + +#include +#include + +#include + +#include +#include + +#include "libgearman/client.hpp" + +static struct OrderRecorder +{ + char buffer[4]; + int pos; +} recorder; + +static gearman_return_t fifo_echo_worker(gearman_job_st* job, void* /*context_arg*/) +{ + /* Echo the workload back as the result (required for the client's + complete callback to receive the original "1"/"2"/"3" strings). */ + return gearman_job_send_data(job, + gearman_job_workload(job), + gearman_job_workload_size(job)); +} + +static gearman_return_t fifo_complete(gearman_task_st *task) +{ + if (recorder.pos < 3) + { + const void *data= gearman_task_data(task); + size_t size= gearman_task_data_size(task); + if (data && size == 1) + { + recorder.buffer[recorder.pos++]= *static_cast(data); + } + } + return GEARMAN_SUCCESS; +} + +/** Test that verifies gearman_packet_create() now builds a FIFO queue + (append-to-tail) instead of LIFO (prepend-to-head). + + This directly exercises the packet linked-list change for issue #395. + Three tasks are submitted in order via the exact batch API path used by + the PHP PECL extension (and all other bindings). Completion order is + recorded via the client-level complete callback. After the patch the + order must be "123"; before the patch it would be "321". */ + +test_return_t fifo_test(void *object) +{ + Context *context= (Context *)object; + ASSERT_TRUE(context); + + const char *function_name= "fifo_echo"; + + /* Start a background worker that will echo the workload (required so + the client receives results and can fire complete callbacks). */ + gearman_function_t echo_fn= gearman_function_create(fifo_echo_worker); + struct worker_handle_st *worker_handle= + test_worker_start(context->port(), + NULL, + function_name, + echo_fn, + NULL, + gearman_worker_options_t()); + ASSERT_TRUE(worker_handle); + + /* Client that will submit the batch of tasks. Note: the Client class lives + in namespace org::gearmand::libgearman. */ + org::gearmand::libgearman::Client client(context->port()); + + /* Reset the shared recorder before submitting tasks. */ + recorder.pos= 0; + + /* Register the client-level complete callback. */ + gearman_client_set_complete_fn(&client, fifo_complete); + + /* Submit tasks in FIFO order using the exact API path that populates + gearman_universal_st::packet_list (the code changed in packet.cc). */ + const char *job_data[3]= {"1", "2", "3"}; + for (int i= 0; i < 3; ++i) + { + gearman_return_t rc; + gearman_task_st* task= + gearman_client_add_task(&client, + NULL, /* let library allocate task */ + NULL, /* not used */ + function_name, + NULL, /* unique */ + job_data[i], + 1, /* workload size */ + &rc); + ASSERT_TRUE(task != NULL); + ASSERT_EQ(GEARMAN_SUCCESS, rc); + } + + /* This is where the packet-list ordering matters: run_tasks() walks + the list and sends packets in the order they were inserted. */ + gearman_return_t ret= gearman_client_run_tasks(&client); + ASSERT_EQ(GEARMAN_SUCCESS, ret); + + /* Shutdown the worker. */ + worker_handle->shutdown(); + delete worker_handle; + + /* === FIFO ASSERTION === */ + /* Before the packet.cc patch: would be "321" (LIFO). */ + /* After the patch: must be "123" (FIFO). */ + ASSERT_EQ(3, recorder.pos); + ASSERT_EQ('1', recorder.buffer[0]); + ASSERT_EQ('2', recorder.buffer[1]); + ASSERT_EQ('3', recorder.buffer[2]); + + return TEST_SUCCESS; +} + +/* ==================================================================== + Required for standalone test binary (provides get_world for libtest) + ==================================================================== */ + +extern "C" void get_world(libtest::Framework *framework) +{ + framework->create(); +} diff --git a/tests/libgearman-1.0/include.am b/tests/libgearman-1.0/include.am index 0d42b9ba..35930c85 100644 --- a/tests/libgearman-1.0/include.am +++ b/tests/libgearman-1.0/include.am @@ -81,8 +81,8 @@ t_signal_wait_LDADD+= libgearman/libgearmancore.la check_PROGRAMS+= t/signal_wait noinst_PROGRAMS+= t/signal_wait -t_worker_LDADD= t_worker_SOURCES= +t_worker_LDADD= t_worker_SOURCES+= libgearman/command.cc t_worker_SOURCES+= tests/libgearman-1.0/worker_test.cc @@ -110,7 +110,6 @@ gdb-signal-wait: t/signal_wait helgrind-internals: t/internals gearmand/gearmand @$(HELGRIND_COMMAND) t/internals - valgrind-internals: t/internals gearmand/gearmand @$(VALGRIND_COMMAND) t/internals @@ -151,3 +150,22 @@ noinst_PROGRAMS+= t/1077917 valgrind-1077917: t/1077917 @$(VALGRIND_COMMAND) --leak-check=full --show-reachable=yes --track-origins=yes t/1077917 + +t_fifo_SOURCES= +t_fifo_LDADD= +t_fifo_CXXFLAGS= +t_fifo_SOURCES+= tests/libgearman-1.0/fifo.cc +t_fifo_LDADD+= ${LIBGEARMAN_1_0_CLIENT_LDADD} +t_fifo_LDADD+= @PTHREAD_LIBS@ +t_fifo_CXXFLAGS+= @PTHREAD_CFLAGS@ +check_PROGRAMS+=t/fifo +noinst_PROGRAMS+=t/fifo + +test-fifo: t/fifo gearmand/gearmand + @t/fifo + +gdb-fifo: t/fifo gearmand/gearmand + @$(GDB_COMMAND) t/fifo + +valgrind-fifo: t/fifo gearmand/gearmand + @$(VALGRIND_COMMAND) t/fifo From 8e4df4803e66e3bd84f00a761be093a1305a99ab Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 21:29:57 -0400 Subject: [PATCH 02/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 28c796d1..65f1909d 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -163,5 +163,5 @@ test_return_t fifo_test(void *object) extern "C" void get_world(libtest::Framework *framework) { - framework->create(); + framework->template create(); } From a3e69b0f2d0419e6a5727856c3ac715870126ffa Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 21:37:22 -0400 Subject: [PATCH 03/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 65f1909d..2e85b4fc 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -163,5 +163,5 @@ test_return_t fifo_test(void *object) extern "C" void get_world(libtest::Framework *framework) { - framework->template create(); + framework->create(new Context()); } From c7c30726b434a45001495f3356e35793ae9e75d8 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 21:56:56 -0400 Subject: [PATCH 04/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 2e85b4fc..5c04637a 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -50,6 +50,8 @@ using namespace libtest; #include #include "libgearman/client.hpp" +#include "libgearman/worker.hpp" +using namespace org::gearmand; static struct OrderRecorder { @@ -108,9 +110,8 @@ test_return_t fifo_test(void *object) gearman_worker_options_t()); ASSERT_TRUE(worker_handle); - /* Client that will submit the batch of tasks. Note: the Client class lives - in namespace org::gearmand::libgearman. */ - org::gearmand::libgearman::Client client(context->port()); + /* Client that will submit the batch of tasks. */ + libgearman::Client client(context->port()); /* Reset the shared recorder before submitting tasks. */ recorder.pos= 0; @@ -156,12 +157,3 @@ test_return_t fifo_test(void *object) return TEST_SUCCESS; } - -/* ==================================================================== - Required for standalone test binary (provides get_world for libtest) - ==================================================================== */ - -extern "C" void get_world(libtest::Framework *framework) -{ - framework->create(new Context()); -} From b1eb91458c650e95fd8a26fadce949346b9932d6 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 22:08:06 -0400 Subject: [PATCH 05/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 5c04637a..e904223c 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -41,8 +41,8 @@ using namespace libtest; -#include #include +#include #include @@ -155,5 +155,11 @@ test_return_t fifo_test(void *object) ASSERT_EQ('2', recorder.buffer[1]); ASSERT_EQ('3', recorder.buffer[2]); + std::cerr << "fifo_test: FIFO packet order verified -> " + << recorder.buffer[0] + << recorder.buffer[1] + << recorder.buffer[2] + << std::endl; + return TEST_SUCCESS; } From 8b39bfaf3d2c83f33e82467a06f609d9b4f83a4e Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 22:09:54 -0400 Subject: [PATCH 06/18] Revise test. --- tests/libgearman-1.0/include.am | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/tests/libgearman-1.0/include.am b/tests/libgearman-1.0/include.am index 35930c85..160fad87 100644 --- a/tests/libgearman-1.0/include.am +++ b/tests/libgearman-1.0/include.am @@ -29,6 +29,7 @@ t_client_SOURCES+= tests/libgearman-1.0/server_options.cc t_client_SOURCES+= tests/libgearman-1.0/task.cc t_client_SOURCES+= tests/libgearman-1.0/unique.cc t_client_SOURCES+= tests/libgearman-1.0/priority_test.cc +t_client_SOURCES+= tests/libgearman-1.0/fifo.cc t_client_SOURCES+= tests/workers/aggregator/cat.cc t_client_SOURCES+= tests/workers/v1/echo_or_react.cc t_client_SOURCES+= tests/workers/v1/echo_or_react_chunk.cc @@ -150,22 +151,3 @@ noinst_PROGRAMS+= t/1077917 valgrind-1077917: t/1077917 @$(VALGRIND_COMMAND) --leak-check=full --show-reachable=yes --track-origins=yes t/1077917 - -t_fifo_SOURCES= -t_fifo_LDADD= -t_fifo_CXXFLAGS= -t_fifo_SOURCES+= tests/libgearman-1.0/fifo.cc -t_fifo_LDADD+= ${LIBGEARMAN_1_0_CLIENT_LDADD} -t_fifo_LDADD+= @PTHREAD_LIBS@ -t_fifo_CXXFLAGS+= @PTHREAD_CFLAGS@ -check_PROGRAMS+=t/fifo -noinst_PROGRAMS+=t/fifo - -test-fifo: t/fifo gearmand/gearmand - @t/fifo - -gdb-fifo: t/fifo gearmand/gearmand - @$(GDB_COMMAND) t/fifo - -valgrind-fifo: t/fifo gearmand/gearmand - @$(VALGRIND_COMMAND) t/fifo From 03f50c73e843d6b5d311386b1d9931ca826db7ea Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 22:32:29 -0400 Subject: [PATCH 07/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index e904223c..6d98b72d 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -155,7 +155,7 @@ test_return_t fifo_test(void *object) ASSERT_EQ('2', recorder.buffer[1]); ASSERT_EQ('3', recorder.buffer[2]); - std::cerr << "fifo_test: FIFO packet order verified -> " + std::cout << "fifo_test: FIFO packet order verified -> " << recorder.buffer[0] << recorder.buffer[1] << recorder.buffer[2] From 98a587ad6a966479bc3952ec219cbf66fa389ef1 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 22:49:17 -0400 Subject: [PATCH 08/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 6d98b72d..ecea2490 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -161,5 +161,7 @@ test_return_t fifo_test(void *object) << recorder.buffer[2] << std::endl; + ASSERT_EQ(1, 0); + return TEST_SUCCESS; } From 4b6583b883b6b23a482b6ff176ab4889cdaa59d8 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 23:16:08 -0400 Subject: [PATCH 09/18] Revise test. --- tests/libgearman-1.0/client_test.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/libgearman-1.0/client_test.cc b/tests/libgearman-1.0/client_test.cc index a871d60e..f6039f1d 100644 --- a/tests/libgearman-1.0/client_test.cc +++ b/tests/libgearman-1.0/client_test.cc @@ -2459,6 +2459,15 @@ test_st limit_tests[] ={ {0, 0, 0} }; +// Forward declaration for fifo test +test_return_t fifo_test(void*); + +test_st fifo_TESTS[] ={ + {"fifo_test", 0, fifo_test }, + {0, 0, 0} +}; + + // Forward declaration for priority tests test_st *test_gearman_worker_priority(void); @@ -2509,6 +2518,7 @@ collection_st collection[] ={ {"fork", fork_SETUP, 0, client_fork_TESTS }, {"loop", 0, 0, loop_TESTS}, {"limits", 0, 0, limit_tests }, + {"fifo task packet order", 0, 0, fifo_TESTS}, {"client-logging", pre_logging, 0, tests_log_TESTS }, {"regression", 0, 0, regression_tests}, {"regression2", reset_SETUP, 0, regression2_TESTS }, From 16ee79410202510f34090f6f0762f95d962751f7 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 23:38:47 -0400 Subject: [PATCH 10/18] Revise test. --- tests/libgearman-1.0/client_test.cc | 4 ++-- tests/libgearman-1.0/fifo.cc | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/libgearman-1.0/client_test.cc b/tests/libgearman-1.0/client_test.cc index f6039f1d..766c7e8e 100644 --- a/tests/libgearman-1.0/client_test.cc +++ b/tests/libgearman-1.0/client_test.cc @@ -2463,7 +2463,7 @@ test_st limit_tests[] ={ test_return_t fifo_test(void*); test_st fifo_TESTS[] ={ - {"fifo_test", 0, fifo_test }, + {"fifo packet order", 0, fifo_test }, {0, 0, 0} }; @@ -2518,7 +2518,7 @@ collection_st collection[] ={ {"fork", fork_SETUP, 0, client_fork_TESTS }, {"loop", 0, 0, loop_TESTS}, {"limits", 0, 0, limit_tests }, - {"fifo task packet order", 0, 0, fifo_TESTS}, + {"fifo", 0, 0, fifo_TESTS}, {"client-logging", pre_logging, 0, tests_log_TESTS }, {"regression", 0, 0, regression_tests}, {"regression2", reset_SETUP, 0, regression2_TESTS }, diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index ecea2490..a41d17ba 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -101,13 +101,13 @@ test_return_t fifo_test(void *object) /* Start a background worker that will echo the workload (required so the client receives results and can fire complete callbacks). */ gearman_function_t echo_fn= gearman_function_create(fifo_echo_worker); - struct worker_handle_st *worker_handle= + std::unique_ptr worker_handle( test_worker_start(context->port(), NULL, function_name, echo_fn, NULL, - gearman_worker_options_t()); + gearman_worker_options_t())); ASSERT_TRUE(worker_handle); /* Client that will submit the batch of tasks. */ @@ -143,9 +143,7 @@ test_return_t fifo_test(void *object) gearman_return_t ret= gearman_client_run_tasks(&client); ASSERT_EQ(GEARMAN_SUCCESS, ret); - /* Shutdown the worker. */ - worker_handle->shutdown(); - delete worker_handle; + /* No manual shutdown or delete needed - unique_ptr handles it. */ /* === FIFO ASSERTION === */ /* Before the packet.cc patch: would be "321" (LIFO). */ @@ -161,7 +159,7 @@ test_return_t fifo_test(void *object) << recorder.buffer[2] << std::endl; - ASSERT_EQ(1, 0); + ASSERT_TRUE(1 == 0); return TEST_SUCCESS; } From b26bd169eda5a3a05de9844e4eeeebda4e589454 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Fri, 3 Apr 2026 23:52:31 -0400 Subject: [PATCH 11/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index a41d17ba..a3d0303d 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -108,7 +108,8 @@ test_return_t fifo_test(void *object) echo_fn, NULL, gearman_worker_options_t())); - ASSERT_TRUE(worker_handle); + + ASSERT_TRUE(worker_handle.get() != NULL); /* Client that will submit the batch of tasks. */ libgearman::Client client(context->port()); @@ -143,7 +144,7 @@ test_return_t fifo_test(void *object) gearman_return_t ret= gearman_client_run_tasks(&client); ASSERT_EQ(GEARMAN_SUCCESS, ret); - /* No manual shutdown or delete needed - unique_ptr handles it. */ + /* No manual shutdown() or delete - unique_ptr cleans up automatically. */ /* === FIFO ASSERTION === */ /* Before the packet.cc patch: would be "321" (LIFO). */ From b892f0b69594a30f885705e2a1bef0f0c018c043 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Sun, 5 Apr 2026 01:46:02 -0400 Subject: [PATCH 12/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index a3d0303d..802396cc 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -160,7 +160,7 @@ test_return_t fifo_test(void *object) << recorder.buffer[2] << std::endl; - ASSERT_TRUE(1 == 0); +// ASSERT_TRUE(1 == 0); // temporary debug line - remove for final PR return TEST_SUCCESS; } From 312b5a54ea179221d4fb0db808067d591ac876c2 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Mon, 6 Apr 2026 18:17:26 -0400 Subject: [PATCH 13/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 45 +++++++++++++++++------------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 802396cc..c06afe12 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -49,10 +49,6 @@ using namespace libtest; #include #include -#include "libgearman/client.hpp" -#include "libgearman/worker.hpp" -using namespace org::gearmand; - static struct OrderRecorder { char buffer[4]; @@ -101,24 +97,25 @@ test_return_t fifo_test(void *object) /* Start a background worker that will echo the workload (required so the client receives results and can fire complete callbacks). */ gearman_function_t echo_fn= gearman_function_create(fifo_echo_worker); - std::unique_ptr worker_handle( - test_worker_start(context->port(), - NULL, - function_name, - echo_fn, - NULL, - gearman_worker_options_t())); - - ASSERT_TRUE(worker_handle.get() != NULL); - - /* Client that will submit the batch of tasks. */ - libgearman::Client client(context->port()); + worker_handle_st *worker_handle = test_worker_start(context->port(), + NULL, + function_name, + echo_fn, + NULL, + gearman_worker_options_t()); + ASSERT_TRUE(worker_handle != NULL); + + /* Use plain C API (exactly like every test in client_test.cc) so we avoid + the libgearman::Client C++ wrapper and its Boost shared_ptr destructor + that was triggering the assertion during teardown. */ + gearman_client_st *client = gearman_client_create(NULL); + ASSERT_TRUE(client != NULL); /* Reset the shared recorder before submitting tasks. */ recorder.pos= 0; /* Register the client-level complete callback. */ - gearman_client_set_complete_fn(&client, fifo_complete); + gearman_client_set_complete_fn(client, fifo_complete); /* Submit tasks in FIFO order using the exact API path that populates gearman_universal_st::packet_list (the code changed in packet.cc). */ @@ -127,7 +124,7 @@ test_return_t fifo_test(void *object) { gearman_return_t rc; gearman_task_st* task= - gearman_client_add_task(&client, + gearman_client_add_task(client, NULL, /* let library allocate task */ NULL, /* not used */ function_name, @@ -141,14 +138,10 @@ test_return_t fifo_test(void *object) /* This is where the packet-list ordering matters: run_tasks() walks the list and sends packets in the order they were inserted. */ - gearman_return_t ret= gearman_client_run_tasks(&client); + gearman_return_t ret= gearman_client_run_tasks(client); ASSERT_EQ(GEARMAN_SUCCESS, ret); - /* No manual shutdown() or delete - unique_ptr cleans up automatically. */ - - /* === FIFO ASSERTION === */ - /* Before the packet.cc patch: would be "321" (LIFO). */ - /* After the patch: must be "123" (FIFO). */ + /* === FIFO verification === */ ASSERT_EQ(3, recorder.pos); ASSERT_EQ('1', recorder.buffer[0]); ASSERT_EQ('2', recorder.buffer[1]); @@ -162,5 +155,9 @@ test_return_t fifo_test(void *object) // ASSERT_TRUE(1 == 0); // temporary debug line - remove for final PR + /* Explicit cleanup (exactly like every other test in client_test.cc) */ + gearman_client_free(client); + test_worker_shutdown(worker_handle); + return TEST_SUCCESS; } From ec8e8aed93f5532eb1383f64487c8a01e811bfee Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Mon, 6 Apr 2026 20:32:33 -0400 Subject: [PATCH 14/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index c06afe12..98558b75 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -105,9 +105,6 @@ test_return_t fifo_test(void *object) gearman_worker_options_t()); ASSERT_TRUE(worker_handle != NULL); - /* Use plain C API (exactly like every test in client_test.cc) so we avoid - the libgearman::Client C++ wrapper and its Boost shared_ptr destructor - that was triggering the assertion during teardown. */ gearman_client_st *client = gearman_client_create(NULL); ASSERT_TRUE(client != NULL); @@ -155,9 +152,9 @@ test_return_t fifo_test(void *object) // ASSERT_TRUE(1 == 0); // temporary debug line - remove for final PR - /* Explicit cleanup (exactly like every other test in client_test.cc) */ + /* Explicit cleanup following the pattern of client_test.cc::loop_test */ gearman_client_free(client); - test_worker_shutdown(worker_handle); + delete worker_handle; return TEST_SUCCESS; } From f145b0ae4bc383d409afb9c9c844438f42347725 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Mon, 6 Apr 2026 20:49:37 -0400 Subject: [PATCH 15/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 98558b75..10789616 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -43,6 +43,7 @@ using namespace libtest; #include #include +#include #include @@ -94,15 +95,15 @@ test_return_t fifo_test(void *object) const char *function_name= "fifo_echo"; - /* Start a background worker that will echo the workload (required so - the client receives results and can fire complete callbacks). */ - gearman_function_t echo_fn= gearman_function_create(fifo_echo_worker); - worker_handle_st *worker_handle = test_worker_start(context->port(), - NULL, - function_name, - echo_fn, - NULL, - gearman_worker_options_t()); + /* Exactly the same pattern used by every other test in client_test.cc */ + gearman_function_t echo_fn = gearman_function_create(fifo_echo_worker); + std::unique_ptr worker_handle( + test_worker_start(context->port(), + NULL, + function_name, + echo_fn, + NULL, + gearman_worker_options_t())); ASSERT_TRUE(worker_handle != NULL); gearman_client_st *client = gearman_client_create(NULL); @@ -154,7 +155,7 @@ test_return_t fifo_test(void *object) /* Explicit cleanup following the pattern of client_test.cc::loop_test */ gearman_client_free(client); - delete worker_handle; + /* unique_ptr destructor runs here and calls the correct shutdown logic */ return TEST_SUCCESS; } From 53ced2369b026c4165e2470a33a58adfe24146f0 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Sun, 14 Jun 2026 23:36:40 -0400 Subject: [PATCH 16/18] Fix fifo_echo_worker to send payload in WORK_COMPLETE packet. --- tests/libgearman-1.0/fifo.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 10789616..666704e7 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -60,9 +60,9 @@ static gearman_return_t fifo_echo_worker(gearman_job_st* job, void* /*context_ar { /* Echo the workload back as the result (required for the client's complete callback to receive the original "1"/"2"/"3" strings). */ - return gearman_job_send_data(job, - gearman_job_workload(job), - gearman_job_workload_size(job)); + return gearman_job_send_complete(job, + gearman_job_workload(job), + gearman_job_workload_size(job)); } static gearman_return_t fifo_complete(gearman_task_st *task) From 9447e07e0770ebcb063de38e2256c80c7496e8eb Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Sun, 14 Jun 2026 23:57:35 -0400 Subject: [PATCH 17/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 666704e7..463508ff 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -59,13 +59,13 @@ static struct OrderRecorder static gearman_return_t fifo_echo_worker(gearman_job_st* job, void* /*context_arg*/) { /* Echo the workload back as the result (required for the client's - complete callback to receive the original "1"/"2"/"3" strings). */ - return gearman_job_send_complete(job, - gearman_job_workload(job), - gearman_job_workload_size(job)); + workload callback to receive the original "1"/"2"/"3" strings). */ + return gearman_job_send_data(job, + gearman_job_workload(job), + gearman_job_workload_size(job)); } -static gearman_return_t fifo_complete(gearman_task_st *task) +static gearman_return_t fifo_workload(gearman_task_st *task) { if (recorder.pos < 3) { @@ -85,7 +85,7 @@ static gearman_return_t fifo_complete(gearman_task_st *task) This directly exercises the packet linked-list change for issue #395. Three tasks are submitted in order via the exact batch API path used by the PHP PECL extension (and all other bindings). Completion order is - recorded via the client-level complete callback. After the patch the + recorded via the client-level workload callback. After the patch the order must be "123"; before the patch it would be "321". */ test_return_t fifo_test(void *object) @@ -112,8 +112,8 @@ test_return_t fifo_test(void *object) /* Reset the shared recorder before submitting tasks. */ recorder.pos= 0; - /* Register the client-level complete callback. */ - gearman_client_set_complete_fn(client, fifo_complete); + /* Register the client-level workload callback. */ + gearman_client_set_workload_fn(client, fifo_workload); /* Submit tasks in FIFO order using the exact API path that populates gearman_universal_st::packet_list (the code changed in packet.cc). */ From a8562ac1c5771229c3c2e6347c436142463971c9 Mon Sep 17 00:00:00 2001 From: Ed Sabol <22986767+esabol@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:13:11 -0400 Subject: [PATCH 18/18] Revise test. --- tests/libgearman-1.0/fifo.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/libgearman-1.0/fifo.cc b/tests/libgearman-1.0/fifo.cc index 463508ff..4eb68876 100644 --- a/tests/libgearman-1.0/fifo.cc +++ b/tests/libgearman-1.0/fifo.cc @@ -139,13 +139,13 @@ test_return_t fifo_test(void *object) gearman_return_t ret= gearman_client_run_tasks(client); ASSERT_EQ(GEARMAN_SUCCESS, ret); - /* === FIFO verification === */ + /* === FIFO verification: wrong === */ ASSERT_EQ(3, recorder.pos); - ASSERT_EQ('1', recorder.buffer[0]); + ASSERT_EQ('3', recorder.buffer[0]); ASSERT_EQ('2', recorder.buffer[1]); - ASSERT_EQ('3', recorder.buffer[2]); + ASSERT_EQ('1', recorder.buffer[2]); - std::cout << "fifo_test: FIFO packet order verified -> " + std::cout << "fifo_test: LIFO packet order verified -> " << recorder.buffer[0] << recorder.buffer[1] << recorder.buffer[2]