-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add itertools.chain.__setstate__ #3779
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
Conversation
@nonzerofloat you may forgot to set |
Generally git do not allow commits without an email registered. That's perhaps because the commit's email is not consistent with my account's email. I have fixed. Correct lint errors. |
vm/src/stdlib/itertools.rs
Outdated
impl IterNextIterable for PyItertoolsChain {} | ||
impl IterNext for PyItertoolsChain { | ||
fn next(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn> { | ||
let iterables = zelf.iterables.read(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change may cause dead-lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give me more detailed explanation for deadlock? I suspect the place is suitable. Deadlock is unexpected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess we have a chain with self-editing iterable on __iter__
. Previously, we locked iterable only before and after get_iter
, it is safe.
After change, this line will hold the lock for entire loop. Any write operation in get_iter will be permanently blocked.
The implementation of |
Oh, this is done in #4232 Thanks @dannasman ! And thank you for contributing @nonzerofloat |
fn setstate(&self, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> { | ||
let args = state.as_slice(); | ||
if args.is_empty() { | ||
let msg = String::from("function takes at leat 1 arguments (0 given)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leat
should be least
.
No description provided.