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.

Sign up for GitHub

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

Jump to bottom

SmbTransport.ClearCachedConnections with force=true leads to InvalidOperationException #40

Open
GoldenCave opened this issue Apr 1, 2021 · 4 comments · May be fixed by #42
Open

SmbTransport.ClearCachedConnections with force=true leads to InvalidOperationException #40

GoldenCave opened this issue Apr 1, 2021 · 4 comments · May be fixed by #42

Comments

@GoldenCave
Copy link

When calling SmbTransport.ClearCachedConnections (true) with force=true, we experienced an InvalidOperationException:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
at SharpCifs.Smb.SmbTransport.ClearCachedConnections(Boolean force)

Here is the code: SmbTransport.ClearCachedConnections. The code enumerates over that collection (SmbConstants.Connections), and then tries to remove an element from it (with force=true), which gives it the InvalidOperationException. At least that is what stackoverflow suggests "You can't remove items from the same list you're using in the foreach, you'll get an exception (System.InvalidOperationException)."
Otherwise this other stack overflow suggests that the collection itself is modified by another thread, but the code above has a lock on the collection, so I think it is the one responsible stack overflow

So, it seems if we changed it to use an index-based for loop instead, we could get around this problem.
@GoldenCave GoldenCave mentioned this issue Apr 1, 2021
Open
@GoldenCave
Copy link
Author

@ume05rw Do you think it would be possible to change this for the next version of SharpsCifs.Std?

@gcamp806
Copy link

We just recently fixed this ourselves. Change this line -

SharpCifs.Std/SharpCifs.STD1.3/Smb/SmbTransport.cs

Line 100 in d74af59

foreach (var transport in SmbConstants.Connections)

to this: foreach (var transport in SmbConstants.Connections.ToArray())

@GoldenCave
Copy link
Author

@gcamp806 Thank you! My team is trying to figure out our next steps and whether we fork this ourselves or if we want to find another solution.

@gcamp806
Copy link

@GoldenCave You're welcome! We decided to fork it and publish an internal NuGet package. There might be other viable solutions out there, but this one has worked well up until this edge case we encountered. The cost of rewriting / testing code to use another solution was more "expensive" than just publishing an internal package once. While doing that, we also added the updates in PR #39.

@GoldenCave GoldenCave linked a pull request Sep 14, 2021 that will close this issue
Open
JSGInray added a commit to JSGInray/SharpCifs.SMBv1 that referenced this issue May 2, 2024
@JSGInray
Apply fix from ume05rw#40 (comment)
9bb603e
BoBiene added a commit to BoBiene/SharpCifs.SMBv1 that referenced this issue May 2, 2024
@BoBiene
Merge pull request #1 from JSGInray/patch-1
76b2c02 
Apply fix from ume05rw#40 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use ToArray() in ClearCachedConnections to prevent InvalidOperationEx… GoldenCave/SharpCifs.Std
2 participants
@gcamp806 @GoldenCave