[WIP] experiment - Use SemaphoreSlim to prevent concurrent basestream access
A spin off / follow up to #589 which tries to use SemaphoreSlim to prevent concurrent access to the base stream instead of lock()
Another followup thought after looking at the bits that lock a bit more:
The TestArchive function:
- calls
TestLocalHeader, which locks the stream for the duration of the operation - reads the stream content via
GetInputStream, which returns the locking PartialInputStream - uses
ZipHelperStreamto read the data descriptor, which I don't think does any locking
so might (3) be a hole where multiple things could read the stream at once?
I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.
Codecov Report
Merging #619 (215f747) into master (1b9fcfc) will increase coverage by
0.05%. The diff coverage is100.00%.
@@ Coverage Diff @@
## master #619 +/- ##
==========================================
+ Coverage 73.35% 73.40% +0.05%
==========================================
Files 68 68
Lines 8718 8735 +17
==========================================
+ Hits 6395 6412 +17
Misses 2323 2323
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs | 78.06% <100.00%> (+0.25%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 1b9fcfc...215f747. Read the comment docs.
I've read elsewhere that you don't need to dispose the semaphore if you aren't using it's wait handle property directly, but I don't know if that's something to depend on.
Extra related question: Should the Async stream calls use ConfigureAwait ?
Regarding ConfigureAwait:
if you’re writing general-purpose library code, use ConfigureAwait(false)
from https://devblogs.microsoft.com/dotnet/configureawait-faq/#when-should-i-use-configureawaitfalse
Yes, just thought i'd check as it can mean adding it in many places, and #574 didn't seem to do it.
No, but that's just my bad 😅