Skip to content
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

Apply pending refcnt updates before release #4106

Closed

Conversation

adamreichold
Copy link
Member

@adamreichold adamreichold commented Apr 21, 2024

Closes #4105

… before release the GIL back to Python code.
@adamreichold adamreichold force-pushed the apply-pending-refcnt-updates-before-release branch 2 times, most recently from 3c70db2 to e3518b9 Compare April 21, 2024 13:36
@davidhewitt
Copy link
Member

I think based on #4105 (comment), this fix probably isn't enough. My test case shown in that comment still crashes.

@adamreichold
Copy link
Member Author

I think based on #4105 (comment), this fix probably isn't enough. My test case shown in that comment still crashes.

Do we still want this for a 0.21.3 bugfix release? It does not fix everything but it should be closer to the behaviour with delayed decrements based on the GIL pool.

@davidhewitt
Copy link
Member

davidhewitt commented Apr 21, 2024

Given that it looks like we can't land a full fix in a patch, sure, let's do this and then I'll see about getting a patch shipped soon 👍

@adamreichold
Copy link
Member Author

I have reconsidered this and I think we should not do this at all: It does not seem to solve the OP's problem, can significantly increase our call overhead, we have to remember to remove this after solving this issue for good and wrapping things in allow_threads should provide a workaround for existing code.

After #4095 is finished, it should not be a correctness issue any more when/where we call update_counts and we can revisit whether we want to call it each and every time or only if a certain amount of updates is pending etc.

@adamreichold adamreichold deleted the apply-pending-refcnt-updates-before-release branch April 22, 2024 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

segmentation fault second run of rust function in python pyo3 0.21.2
2 participants