From 97407cd11ec3febd1d70ac61c09d8c0cef3bc8c8 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 23 Jun 2026 15:39:36 -0700 Subject: [PATCH] client: fix Op early destruction Errors resulting in early Op destruction would erroneously be escalated to "Not in running evbase worker" exceptions. Something has already gone wrong, but this would mask the root cause. --- src/clientdiscover.cpp | 4 +++- src/clientget.cpp | 3 ++- src/clientimpl.h | 2 ++ src/clientmon.cpp | 3 ++- test/testget.cpp | 13 ++++++++++++- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/clientdiscover.cpp b/src/clientdiscover.cpp index 59721ed..f9dc438 100644 --- a/src/clientdiscover.cpp +++ b/src/clientdiscover.cpp @@ -21,7 +21,7 @@ Discovery::Discovery(const std::shared_ptr &context, const std::str {} Discovery::~Discovery() { - if(loop.assertInRunningLoop()) + if(onWorker && loop.assertInRunningLoop()) _cancel(true); } @@ -81,6 +81,8 @@ std::shared_ptr DiscoverBuilder::exec() // setup timer to send discovery context->tcp_loop.dispatch([op, context, ping]() { + // on worker + op->onWorker = true; if(context->state!=ContextImpl::Running) throw std::logic_error("Context close()d"); diff --git a/src/clientget.cpp b/src/clientget.cpp index adaf396..59320b4 100644 --- a/src/clientget.cpp +++ b/src/clientget.cpp @@ -138,7 +138,7 @@ struct GPROp : public OperationBase :OperationBase (op, loop, name) {} ~GPROp() { - if(loop.assertInRunningLoop()) + if(onWorker && loop.assertInRunningLoop()) _cancel(true); } @@ -605,6 +605,7 @@ std::shared_ptr gpr_setup(const std::shared_ptr& context context->tcp_loop.dispatch([internal, context, name, server]() { // on worker + internal->onWorker = true; try { internal->chan = Channel::build(context, name, server); diff --git a/src/clientimpl.h b/src/clientimpl.h index b15fff9..0047a63 100644 --- a/src/clientimpl.h +++ b/src/clientimpl.h @@ -50,6 +50,8 @@ struct OperationBase : public Operation uint32_t ioid = 0; Value result; bool done = false; + // state of handoff from user thread to worker + bool onWorker = false; std::shared_ptr waiter; OperationBase(operation_t op, const evbase& loop, const std::string& name); diff --git a/src/clientmon.cpp b/src/clientmon.cpp index fc45dc8..af5c422 100644 --- a/src/clientmon.cpp +++ b/src/clientmon.cpp @@ -84,7 +84,7 @@ struct SubscriptionImpl final : public OperationBase, public Subscription event_new(loop.base, -1, EV_TIMEOUT, &tickAckS, this)) {} virtual ~SubscriptionImpl() { - if(loop.assertInRunningLoop()) + if(onWorker && loop.assertInRunningLoop()) _cancel(true); } @@ -830,6 +830,7 @@ std::shared_ptr MonitorBuilder::exec() auto server(std::move(_server)); context->tcp_loop.dispatch([op, context, server]() { // on worker + op->onWorker = true; try { op->chan = Channel::build(context, op->channelName, server); diff --git a/test/testget.cpp b/test/testget.cpp index 7ddb336..dd35293 100644 --- a/test/testget.cpp +++ b/test/testget.cpp @@ -25,6 +25,16 @@ namespace { using namespace pvxs; +void testEmptyName() { + client::Config conf; + conf.autoAddrList = false; + auto ctx(conf.build()); + testThrowsMatch("Empty channel name", + [&](){ + ctx.get("").exec(); + }); +} + struct Tester { Value initial; server::SharedPV mbox; @@ -592,10 +602,11 @@ void testServerClose() MAIN(testget) { - testPlan(67); + testPlan(68); testSetup(); logger_config_env(); const bool canIPv6 = pvxs::impl::evsocket::canIPv6; + testEmptyName(); Tester().testConnector(); Tester().testWaiter(); Tester(AF_INET).loopback();