Re: timeout value overflow in wait for lsn

On Thu, Aug 20, 2026 at 11:12 AM cca5507 <cca5507(at)qq(dot)com> wrote:
>
> > Changing the check to FLOAT8_FITS_IN_INT32 would reject larger values.
> > If we choose that limit, the timeout variable and the WaitForLSN()
> > argument should also use int so that the interface matches the
> > accepted range.
> >
> > Alternatively, we can preserve the int64 interface and check the
> > deadline calculation in WaitForLSN():
> >
> > if (pg_mul_s64_overflow(timeout, USECS_PER_MSEC, &timeout_us) ||
> > pg_add_s64_overflow(now, timeout_us, &endtime) ||
> > !IS_VALID_TIMESTAMP(endtime))
> >
> > -----------------
> >
> > Another option is to reject values greater than or equal with
> > INT64_MAX/1000, which seems a bit hacky to me.
>
> The max timeout value supported by WaitLatch() is INT_MAX, so I think
> it's reasonable to limit the range to int32.

We have a loop in the wait infra, which means that the waiter could
fall asleep several times. Each time takes a INT_MAX maximumly, added
up toward a value larger than INT_MAX. That is why I was wondering
whether the bug itself warrants a truncation from 64 to 32. If there
are user needs like absurdly long timeouts, then we better satisfy
them and there're ways to do so. But in my poor imagination, waiting
greater than 25 days seems unlikely in reality. So I voted for the
limitation of the range.

> And I think it's ok to use int64
> as the argument in WaitForLSN() because convert int32 to int64 is always
> safe.

Yeah, it is safe only if we handle the checking/rejection properly.
The wait for infra is also used by repack, which uses timeout as zero
for an indefinite wait and 100 milliseconds for a finite wait. So it
is not vulnerable to edge cases like this. But the infra could have
more potential callers in the future, we better not let them bother
the extra safety if we can deal with it easily. Sorry if this seems
nitpicking to you.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.