CHIA-3899 Release the GIL when calling BlockBuilder's add_spend_bundles#1361
CHIA-3899 Release the GIL when calling BlockBuilder's add_spend_bundles#1361AmineKhaldi wants to merge 1 commit intoChia-Network:mainfrom
Conversation
AmineKhaldi
added
the
Changed
AmineKhaldi
force-pushed
the
add_spend_bundles_drop_gil
branch
from
9ede6bb to
7d65920
Compare
Pull Request Test Coverage Report for Build 22314709672Details
💛 - Coveralls |
AmineKhaldi
changed the title
arvidn
left a comment
arvidn
left a comment
There was a problem hiding this comment.
looks good. I'm a little bit worried that this is going to be a bit slower, but probably doesn't matter much. I suppose we don't have any python benchmark that exercises this
AmineKhaldi
force-pushed
the
add_spend_bundles_drop_gil
branch
2 times, most recently
from
3abc0cc to
fa4815c
Compare
AmineKhaldi
force-pushed
the
add_spend_bundles_drop_gil
branch
from
fa4815c to
d3f2f11
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| .iter() | ||
| .map(|sb| Ok(sb.extract()?)) | ||
| .collect::<PyResult<_>>()?; | ||
| let (added, result) = py.detach(|| self.add_spend_bundles(sbs, cost, constants))?; |
There was a problem hiding this comment.
GIL release can trigger PyCell borrow errors
Medium Severity
Releasing the GIL around BlockBuilder::add_spend_bundles while holding &mut self (and constants borrowed from Python) can allow other Python threads to run and attempt to use the same BlockBuilder concurrently, which may now fail with a PyCell “already borrowed” runtime error instead of being serialized by the GIL.
|
it's not clear what this achieves, given that we call this from the main python thread anyway |
|
I moved this to draft as I don't think we should land this without some more discussion |
2 participants
Note
Low Risk
Binding-layer change that only affects Python threading behavior; minimal logic changes beyond input extraction and running the Rust call without the GIL.
Overview
Updates the
BlockBuilderPython binding foradd_spend_bundlesto release the Python GIL while executing the underlying Rustadd_spend_bundleslogic.The wrapper now collects the input
PyListinto aVec<SpendBundle>with properPyResulterror propagation, then runs the block-building work insidepy.detach(...)to avoid blocking other Python threads.Written by Cursor Bugbot for commit d3f2f11. This will update automatically on new commits. Configure here.