Skip to content

Comments

fix: < should be >#1233

Open
cuiweixie wants to merge 1 commit intomicrosoft:mainfrom
cuiweixie:bugfix
Open

fix: < should be >#1233
cuiweixie wants to merge 1 commit intomicrosoft:mainfrom
cuiweixie:bugfix

Conversation

@cuiweixie
Copy link

@cuiweixie cuiweixie commented Feb 22, 2026

According to:

if (!force && (expire == 0 || expire > now)) return false;

should be >, not <.

@res2k
Copy link
Contributor

res2k commented Feb 22, 2026

Are you sure?
Currently, mi_arenas_purge_expire contains a point in time after which arenas should be "purged".
So the condition in question will leave the function (and not do any purging) until that point in time is reached.

With your change, arenas will be purged if the time is not yet at mi_arenas_purge_expire; given that this is usually a time point in the future most of the time, and also updated to a time point in the future when purging, this means purging will happen continuously.
This is probably not how this is intended to work.

@cuiweixie cuiweixie closed this Feb 22, 2026
@will-extrahop
Copy link

@res2k are you sure you're sure? 🙂 Changing to > would mean that we bail out when arenas_expire > now, i.e. when the expiration time has not yet arrived, which is what we want. It would also match the analogous condition in mi_arena_try_purge() on line 565.

@cuiweixie cuiweixie reopened this Feb 22, 2026
@res2k
Copy link
Contributor

res2k commented Feb 23, 2026

@res2k are you sure you're sure?

Uh, well... certainly less now.

In any case, this discussion shed more light on why there may be an issue than the initial PR description, so that's a win.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants