> 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.
Make sense to me. The v2 patch forgets to update the type of the timeout variable in repack worker, fixed in v3. -- Regards, ChangAo Chen
v3-0001-Fix-overflow-in-WAIT-FOR-LSN-timeout-handling.patch
Description: Binary data
