Author: brane
Date: Fri Jul 18 13:45:41 2025
New Revision: 1927309
URL: http://svn.apache.org/viewvc?rev=1927309&view=rev
Log:
Add context-specific task cleanup when a context is destroyed.
* src/resolve.c: Update top-level TODO docstring.
(do_init_work_queue): Renamed from init_work_queue().
(init_work_queue): New; this is now the init-once entry point.
(cleanup_resolve_tasks): New, pool cleanup function.
(create_resolve_context): Call init_work_queue() and register the cleanup.
(resolve_address_async): Call init_work_queue().
* test/test_context.c
(async_resolve_cancel_callback,
test_async_resolve_cancel): New test for the early cancellation path.
(test_context): Register test_async_resolve_cancel.
Modified:
serf/trunk/src/resolve.c
serf/trunk/test/test_context.c
Modified: serf/trunk/src/resolve.c
URL:
http://svn.apache.org/viewvc/serf/trunk/src/resolve.c?rev=1927309&r1=1927308&r2=1927309&view=diff
==============================================================================
--- serf/trunk/src/resolve.c (original)
+++ serf/trunk/src/resolve.c Fri Jul 18 13:45:41 2025
@@ -50,10 +50,6 @@
/*
* FIXME: EXPERIMENTAL
* TODO:
- * - Add cleanup function for in-flight resolve tasks if their owning
- * context is destroyed. This function should be called from the
- * context's pool cleanup handler.
- *
* - Wake the poll/select in serf_context_run() when new resolve
* results are available.
*
@@ -424,9 +420,10 @@ static apr_status_t run_async_resolver_l
#define MAX_WORK_QUEUE_THREADS 50
static apr_pool_t *work_pool = NULL;
static apr_thread_pool_t *work_queue = NULL;
-static apr_status_t init_work_queue(void *baton)
+
+static apr_status_t do_init_work_queue(void *baton)
{
- serf_context_t *ctx = baton;
+ serf_context_t *const ctx = baton;
apr_status_t status;
apr_pool_create(&work_pool, NULL);
@@ -440,11 +437,29 @@ static apr_status_t init_work_queue(void
return status;
}
+static apr_status_t init_work_queue(serf_context_t *ctx)
+{
+ SERF__DECLARE_STATIC_INIT_ONCE_CONTEXT(init_ctx);
+ return serf__init_once(&init_ctx, do_init_work_queue, ctx);
+}
+
+
+static apr_status_t cleanup_resolve_tasks(void *baton)
+{
+ /* baton is serf_context_t */
+ return apr_thread_pool_tasks_cancel(work_queue, baton);
+}
static apr_status_t create_resolve_context(serf_context_t *ctx)
{
+ apr_status_t status;
+
ctx->resolve_context = NULL;
- return APR_SUCCESS;
+ status = init_work_queue(ctx);
+ if (status == APR_SUCCESS)
+ apr_pool_pre_cleanup_register(ctx->pool, ctx, cleanup_resolve_tasks);
+
+ return status;
}
@@ -507,10 +522,7 @@ static apr_status_t resolve_address_asyn
apr_pool_t *scratch_pool)
{
resolve_task_t *task;
- apr_status_t status;
- SERF__DECLARE_STATIC_INIT_ONCE_CONTEXT(init_ctx);
-
- status = serf__init_once(&init_ctx, init_work_queue, ctx);
+ apr_status_t status = init_work_queue(ctx);
if (status)
return status;
Modified: serf/trunk/test/test_context.c
URL:
http://svn.apache.org/viewvc/serf/trunk/test/test_context.c?rev=1927309&r1=1927308&r2=1927309&view=diff
==============================================================================
--- serf/trunk/test/test_context.c (original)
+++ serf/trunk/test/test_context.c Fri Jul 18 13:45:41 2025
@@ -1058,6 +1058,44 @@ static void test_async_resolve(CuTest *t
handler_ctx, tb->pool);
}
+static void async_resolve_cancel_callback(serf_context_t *ctx,
+ void *resolved_baton,
+ apr_sockaddr_t *host_address,
+ apr_status_t status,
+ apr_pool_t *scratch_pool)
+{
+ *(int*)resolved_baton = 1;
+}
+
+static void test_async_resolve_cancel(CuTest *tc)
+{
+ test_baton_t *tb = tc->testBaton;
+ serf_context_t *ctx;
+ apr_pool_t *ctx_pool;
+ apr_status_t status;
+ apr_uri_t url;
+ int resolved = 0;
+
+ status = apr_uri_parse(tb->pool, "http://localhost:8080/";, &url);
+ CuAssertIntEquals(tc, APR_SUCCESS, status);
+
+ apr_pool_create(&ctx_pool, tb->pool);
+ CuAssertPtrNotNull(tc, ctx_pool);
+ ctx = serf_context_create(ctx_pool);
+ CuAssertPtrNotNull(tc, ctx);
+
+ status = serf_address_resolve_async(ctx, url,
+ async_resolve_cancel_callback,
+ &resolved, ctx_pool);
+
+ /* This would create and actual race in the test case: */
+ /* serf_context_prerun(ctx); */
+
+ apr_pool_destroy(ctx_pool);
+ CuAssertIntEquals(tc, APR_SUCCESS, status);
+ CuAssertIntEquals(tc, 0, resolved);
+}
+
/*****************************************************************************/
CuSuite *test_context(void)
@@ -1086,5 +1124,6 @@ CuSuite *test_context(void)
SUITE_ADD_TEST(suite, test_max_keepalive_requests);
SUITE_ADD_TEST(suite, test_outgoing_request_err);
SUITE_ADD_TEST(suite, test_async_resolve);
+ SUITE_ADD_TEST(suite, test_async_resolve_cancel);
return suite;
}
