SharpCifs.Std icon indicating copy to clipboard operation
SharpCifs.Std copied to clipboard

SmbTransport.ClearCachedConnections with force=true leads to InvalidOperationException

Open GoldenCave opened this issue 3 years ago • 4 comments

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 avatar Apr 01 '21 15:04 GoldenCave

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

GoldenCave avatar Apr 01 '21 15:04 GoldenCave

We just recently fixed this ourselves. Change this line - https://github.com/ume05rw/SharpCifs.Std/blob/d74af592ac052750ab56c91183f5319d58a93031/SharpCifs.STD1.3/Smb/SmbTransport.cs#L100 to this: foreach (var transport in SmbConstants.Connections.ToArray())

gcamp806 avatar Apr 20 '21 21:04 gcamp806

@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.

GoldenCave avatar Apr 22 '21 16:04 GoldenCave

@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 https://github.com/ume05rw/SharpCifs.Std/pull/39.

gcamp806 avatar Apr 22 '21 16:04 gcamp806