- Notifications
You must be signed in to change notification settings - Fork748
When IterableWrapper iterates up to the middle of list, System.AccessViolationException occurs.#2280
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Great catch, thank you very much! |
lostmsu requested changesNov 7, 2023
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.
Please, revert whitespace changes (wrong line breaks?)
… the list before reaching end.
I amended the last commit with 'right' line breaks. Did it work propertly? Sorry I'm not used to this 'push request' workflow. |
lostmsu approved these changesNov 7, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
System.AccessViolationException occurs when iteration quits in the middle of list before reaching the end.
Environment
Reproduce the problem
I'm trying to pass a list from python to .NET method via List Codec.
Conversion(decode) is successful and mylist is wrapped byListWrapper.
.NET code receives the list like this.
Using Enumerator ofmylist, if enumerated to the end of the list, nothing bad happens.
However if iteration didn't go to the end of the list, AccessViolationException is thrown.
This case happens in the following code:
similarly
The following code doesn't throw exception becuase it always iterates to the end of the list.
Exception occurs because PyIter(ator) object is Dispose(d) outside of GIL protection only when iteration ends before reaching the end ( before MoveNext() returns null ).
In such a case, code process just slips out of GIL enclosure as the nature of C# scope and yield return and break.
solution for now
original code is like this:
using var _ = iterObject;
above the while loop is the problematic line.Before iteration reaches the end, the executing process is somewhere out of theusing scope.
When c# caller process stops iteration(Enumeration) for some reason, c# suddely calls
using var _ = iterObject;
's closing process which is iterObject.Dispose().But here iterObject is not inside of GIL() protection. This is why exception occurs.
Solution is to make sure iterObject.Dispose() is always inside of GIL protection and
always executed whatever the caller process abandons the enumeration in the middle.
My suggestion is to use try catch pattern. Without catching any exception it may look odd. But this is one solution to make sure the execution of Dispose and GIL scope surrounding always.