Re: two statement_timeout race conditions

> On Nov 9, 2026, at 08:12, Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>
> Hi,
>
> While running a stress test on a patch I'm working on, I ran into two
> annoying race conditions with statement_timeout. It's not a huge issue,
> most people either don't have statement_timeout set at all, or there's
> sufficient margin to not hit the timeout very often. I've since
> mitigated that in my test script, but it was annoying and it took me a
> while to figure out what's going to (and that it's not a bug in the new
> code I was testing).
>
>
> 1) statement_timeout can kill the next command instead
>
> Roughly what happens is that the command takes just about the timeout,
> the timeout fires, but the statement does not check interrupts before it
> ends, and we leave the cancel request as pending. Then we run the next
> command (which could be as simple as "SET guc"), and that gets killed by
> the timeout. That's exactly what I saw with statement_timeout = '60s',
> SET statements getting aborted.
>
> This happens because isable_statement_timeout() does nothing if
> get_timeout_active() says the timeout is no longer active, which is the
> case once it has fired. So the indicator stays set, and then is reported
> as a statement timeout against whatever comes next (without being
> executed at all).
>
> I'm attaching a SQL reproducer for that (repro-exec.sql), leveraging the
> fact that lpad() does not check interrupts even for very long values
> (which we should probably fix, "repeat" already does that). And
> CALL/FETCH don't either.
>
> The attached 0001 fixes this race by doing pretty much the same thing as
> ProcessInterrupts() for QueryCancelPending - checking both lock/timeout
> indicators, and resetting the flag. With this the reproducer no longer
> reproduces the unexpected interruptions.
>
>
> 2) statement timeout reported as "user request" cancellation
>
> While looking at the code and trying to come up with reproducers for the
> first issue, I ran into a second race, where we correctly cancel the
> query due to a timeout, but report it happened due to a "user request".
>
> It can happen if the timeout expires while parsing a long query string
> (yes, that's not very common), because that does not check interrupts.
> In exec_simple_query() we start the timer, do parsing etc., and then
> call enable_statement_timeout() again, which does this:
>
> if (!get_timeout_active(STATEMENT_TIMEOUT) &&
> !get_timeout_indicator(STATEMENT_TIMEOUT, false))
> enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
>
> But get_timeout_active(() returns false if the timer already fired, so
> the timer gets restarted, but leaves QueryCancelPending set. And so on
> the next CHECK_FOR_INTERRUPTS() the query gets cancelled with the
> misleading "canceling statement due to user request" reason.
>
> I'm attaching a SQL reproducer for this too (repro-parse.sql), hitting
> this. It's a bit artificial, I don't think I've seen this in practice
> (even though some of the queries the stress test produces are long and
> complex). 0002 fixes that by not restarting the timer that has fired
> (which relies on the previous commit).
>
>
> regards
>
> --
> Tomas Vondra
> <v1-0002-Don-t-restart-a-statement-timeout-that-has-alread.patch><v1-0001-Discard-pending-timeout-cancel-requests-at-end-of.patch><repro-parse.sql><repro-exec.sql>

Using your repro scripts, I can reproduce the race on my MacBook too, and I have confirmed that the fix resolves it.

I have a question. This patch seems to add a restriction to disable_statement_timeout(): because it now also discards pending LOCK_TIMEOUT and STATEMENT_TIMEOUT cancellations, it should only be called at a command-completion boundary. Currently, there are only three callers, and they meet this restriction. However, I’m not sure whether a future caller might call this function midway through a statement. From this perspective, I think a new name, for example, finish_statement_timeout(), might avoid confusion.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/