8000 Avoid re-entrant calls to GC collector's callback by akx · Pull Request #343 · prometheus/client_python · GitHub
[go: up one dir, main page]

Skip to content

Avoid re-entrant calls to GC collector's callback #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 16, 2018

Conversation

akx
Copy link
Contributor
@akx akx commented Nov 13, 2018

It looks like the Python GC engine can call the GC callback
while it's still being called (likely if the callback does significant
enough work), which may, if you're unlucky enough, lead into a stack
overflow Python is unable to recover from.

This patch should fix it, though no empirical tests have been made.

Refs this issue comment: #322 (comment)

It looks like the Python GC engine can call the GC callback
while it's still being called (likely if the callback does significant
enough work), which may, if you're unlucky enough, lead into a stack
overflow Python is unable to recover from.

This patch should fix it, though no empirical tests have been made.

Signed-off-by: Aarni Koskela <akx@iki.fi>
@brian-brazil
Copy link
Contributor

We should probably also try to avoid allocating memory in this function, such as by caching the children. That'll reduce the amount of data we lose due to this.

@akx
Copy link
Contributor Author
akx commented Nov 16, 2018

Is that worth the added complexity? Either way, maybe another issue should be made for that, and this merged so people don't have their Pythons fatally crashing...

@brian-brazil brian-brazil merged commit 33bc65b into prometheus:master Nov 16, 2018
@brian-brazil
Copy link
Contributor

Is that worth the added complexity?

It'd avoid losing data on all bar the first GC, and be a little faster too.

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.

2 participants
0