Skip to content

Conversation

@fulghum
Copy link
Contributor

@fulghum fulghum commented Mar 19, 2025

Callers can override regex.RegexLeakHandler to supply their own custom function to be invoked when finalizing a regex.Regex instance that hasn't had its Close() method called.

For this initial version, the callback function doesn't take any arguments, but we could supply the Regex instance as an argument and add a String() method to the Regex interface to allow the caller to see more information on which Regex instance was leaked.

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but @Hydrocharged should take a peek when he has a chance (after merge).

} else if ShouldPanic {
panic("Finalizer found a Regex that was never closed")
}
_ = pr.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know of any reason this wouldn't be safe to do at finalization time, but Daylon should give his opinion here too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking through the code a bit, I can't find nor think of a good reason as to why this shouldn't be fine, so I approve.

@fulghum fulghum merged commit 41d869d into main Mar 19, 2025
3 checks passed
@fulghum fulghum deleted the fulghum/regex branch March 19, 2025 23:23
Copy link
Collaborator

@Hydrocharged Hydrocharged left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks fine to me, no additional comments!

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.

4 participants