squid icon indicating copy to clipboard operation
squid copied to clipboard

WIP: Preserve caller context across event.h callbacks

Open eduard-bagdasaryan opened this issue 4 years ago • 2 comments

eventAdd*() API queues callbacks without saving the call context. Thus, EventScheduler::checkEvents() schedules event callbacks (via AsyncCall API) without restoring the original recipient context. We must preserve the code context across events.

eduard-bagdasaryan avatar Jan 31 '20 12:01 eduard-bagdasaryan

FYI: This PR may need to be reworked to avoid leaking transaction context onto unrelated service code (and, in some cases, preserving unrelated transaction context inside that service forever). Most eventAdd() events are not transaction-specific, but many eventAdd() calls are executed in a transaction context. For example, think of some periodic service cleanup event triggered by the first transaction using that service. I asked Eduard to dig deeper and will review once his investigation is over.

rousskov avatar Feb 02 '20 16:02 rousskov

I suspect the simplest change would be to make events use a sub-context such as "event Foo triggered by CurrentContext()".

yadij avatar Feb 03 '20 00:02 yadij

The vast majority of the existing eventAdd() and eventAddIsh() callers are context-free global services and should stay that way. Eduard is now making good progress with identifying the few eventAdd() callers that need special treatment. This PR will change to restrict context preservation to those few special cases.

No special cases left AFAICT. Closing this stale PR as currently unnecessary. We may come back to this later if we decide to add "service code context" support for various global services.

rousskov avatar Oct 26 '23 17:10 rousskov