RTDPublisher Stop() being called despite remaining active subscribers

[andyvandy]

Happy New Year Steven!

I've run into an edge case where stop() is getting called on Python RTDPublisher instances when a cell disconnects which cuts the connection for other remaining subscribers. ( shutting down a shared service/database connection )
Observed on 0.18.7 py 3.11 64bit.

I haven't gotten a debugger hooked up to the C++ code but this function looked like a plausible trigger.

xloil/src/xlOil-COM/RtdAsyncManager.cpp

Lines 107 to 119 in ac9b361

	bool disconnect(size_t numSubscribers) override
	{
	RtdPublisher::disconnect(numSubscribers);
	// TODO: check numSubscribers == 0
	stop();
	auto p = _parent.lock();
	if (p)
	{
	scoped_atomic_flag lock(p->busy);
	p->tasks.remove_if([&](auto& t) { return t.get() == this; });
	}
	return true;
	}

Assuming that the underlying xlo.RTDPublisher has properly implemented disconnect and stop functions, is there any need for line 111 at all? Even if predicated on numSubscribers==0, would there be a reason to call stop() here rather than from the disconnect function?

If not, the fix looks like it may just be a single line deletion 🤞

[cunnane]

The class here AsyncTaskPublisher is only used to handle the case of an async function running in a cell as RTD. The publisher should have at most 1 subscriber and when the subscriber disconnects it should mean that the formula in the cell was changed, so it should be safe to call stop in this case. However, I have removed the stop in the latest code base as it should already have been called by the disconnect on the previous line as you note.

It sounds like the publisher you are having troubles with is the generic one. In v0.20 I've enable trace level logging in release builds, so you could enable this and see if there are any clues: you should get a fair amount of connection/disconnection info coming from the RtdServerThreadedWorker class. If possible, add a higher level log line to your stop() function so that it's easier to find in the logs. The RTD logic is quite complex so it's very plausible that something goes awry in certain cases.

Happy new year and btw, you commits made it to v0.20, thanks!

[andyvandy]

Thanks! I'll take a look and confirm next week whether this resolves the issue observed or was just a red herring. If it's elsewhere I'll start with assembling a repro then looking into that logging functionality to see it there's a smoking gun.

[andyvandy]

Took a bit longer than I expected to look into this. I think it's still occurring so I'm going to take some time to cobble together a reproducible example. ( don't waste time looking in the meantime!)

Maybe should ask in a separate comment but any chance you could release the current master as v0.20.1 when you get a chance?

[cunnane]

@andyvandy 0.20.1 has been released