Skip to content

Commit cc7bc72

Browse files
committed
client: log error when syncCancel op dtor on worker
Log an error when an operation created with syncCancel=true is allowed to be free'd asynchronously on the client worker. This may indicate that the Operation/Subscription is directly owned by its own callback. Creating a self-reference loop. Or it might indicate a more complicated, but safe, situation like pvalink where a new operation is initiated as the previous one completes. Use the syncCancel=false flag, opting of blocking cancellation, as an indication that this risk is accepted by user code.
1 parent a79d65b commit cc7bc72

7 files changed

Lines changed: 31 additions & 0 deletions

File tree

ioc/pvalink_channel.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ void pvaLinkChannel::put(bool force)
267267
// start net Put, cancels in-progress put
268268
op_put = linkGlobal->provider_remote.put(key.first)
269269
.rawRequest(pvReq)
270+
.syncCancel(false)
270271
.build([this](Value&& prototype) -> Value
271272
{
272273
return linkBuildPut(this, std::move(prototype)); // TODO

src/clientdiscover.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ std::shared_ptr<Operation> DiscoverBuilder::exec()
9393
// (maybe) user thread
9494
auto loop(op->context->tcp_loop);
9595
auto temp(std::move(op));
96+
if(syncCancel && loop.inLoop())
97+
log_err_printf(io,
98+
"syncCancel discover '%s' being destroyed on worker thread.\n"
99+
"Possible self-reference loop. Setup with .syncCancel(false) if intended.\n",
100+
temp->channelName.c_str());
101+
96102
loop.tryInvoke(syncCancel, std::bind([](std::shared_ptr<Discovery>& op){
97103
// on worker
98104
op->context->discoverers.erase(op.get());

src/clientget.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,12 @@ std::shared_ptr<Operation> gpr_setup(const std::shared_ptr<ContextImpl>& context
613613
// (maybe) user thread
614614
auto temp(std::move(internal));
615615
auto loop(temp->loop);
616+
if(syncCancel && loop.inLoop())
617+
log_err_printf(io,
618+
"syncCancel op%u '%s' being destroyed on worker thread.\n"
619+
"Possible self-reference loop. Setup with .syncCancel(false) if intended.\n",
620+
temp->op, temp->channelName.c_str());
621+
616622
// std::bind for lack of c++14 generalized capture
617623
// to move internal ref to worker for dtor
618624
loop.tryInvoke(syncCancel, std::bind([](std::shared_ptr<GPROp>& op) {

src/clientintrospect.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,12 @@ std::shared_ptr<Operation> GetBuilder::_exec_info()
207207
// from user thread
208208
auto temp(std::move(op));
209209
auto loop(temp->loop);
210+
if(syncCancel && loop.inLoop())
211+
log_err_printf(io,
212+
"syncCancel info '%s' being destroyed on worker thread.\n"
213+
"Possible self-reference loop. Setup with .syncCancel(false) if intended.\n",
214+
temp->channelName.c_str());
215+
210216
// std::bind for lack of c++14 generalized capture
211217
// to move internal ref to worker for dtor
212218
loop.tryInvoke(syncCancel, std::bind([](std::shared_ptr<InfoOp>& op) {

src/clientmon.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,12 @@ std::shared_ptr<Subscription> MonitorBuilder::exec()
830830
// from user thread
831831
auto temp(std::move(op));
832832
auto loop(temp->loop);
833+
if(syncCancel && loop.inLoop())
834+
log_err_printf(io,
835+
"syncCancel monitor '%s' being destroyed on worker thread.\n"
836+
"Possible self-reference loop. Setup with .syncCancel(false) if intended.\n",
837+
temp->channelName.c_str());
838+
833839
// std::bind for lack of c++14 generalized capture
834840
// to move internal ref to worker for dtor
835841
loop.tryInvoke(syncCancel, std::bind([](std::shared_ptr<SubscriptionImpl>& op) {

src/evhelper.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,11 @@ bool evbase::_call(mfunction&& fn, bool dothrow) const
348348
return true;
349349
}
350350

351+
bool evbase::inLoop() const
352+
{
353+
return pvt->worker.isCurrentThread();
354+
}
355+
351356
void evbase::assertInLoop() const
352357
{
353358
if(!pvt->worker.isCurrentThread()) {

src/evhelper.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ struct PVXS_API evbase {
198198
return tryDispatch(std::move(fn));
199199
}
200200

201+
bool inLoop() const;
201202
void assertInLoop() const;
202203
//! Caller must be on the worker, or the worker must be stopped.
203204
//! @returns true if working is running.

0 commit comments

Comments
 (0)