mirror of
https://github.com/JustArchiNET/ArchiSteamFarm.git
synced 2026-01-13 23:20:37 +00:00
a8045ac50b
Now this is a nice bug that was found accidentally by ArchiBoT...
ReaderWriterLockSlim() is very decent solution, but it's thread-based, and we're using our ConcurrentHashSet in mixed async/sync context. This means that if we use something like:
foreach (var item in concHashSet) {
await AnythingAsync().ConfigureAwait(false);
}
It's totally possible that we'll request read lock as thread 1, and release the read lock as thread 2, which will lead to RWLock exception => System.Threading.SynchronizationLockException: The read lock is being released without being held.
Fortunately it looks like we didn't have any scenario like this in ASF, as this was possible only when we async/await while enumerating over ConcurrentHashSet, so that specific bug didn't affect ASF codebase (yet). Still, I must fix this as current implementation is not thread-safe, so our HashSet is in fact not concurrent in the first place.
I analyzed possible solutions and there are basically 3: either using ConcurrentDictionary and wrapping around it, replacing lock with SemaphoreSlim, or using third-party AsyncReaderWriterLock from StephenCleary. SemaphoreSlim entirely kills the concept of multiple readers one writer, and could affect performance negatively, moreover - it doesn't support upgreadable lock scenario we have with ReplaceIfNeededWith(). Concurrent dictionary would be nice if I didn't have that awful memory hit from storing mandatory pointless value, plus I don't really like concept of wrapping around conc dictionary if I can simply use it right away and drop my conc hashset entirely. AsyncReaderWriterLock seem to be really well written, and works on Mono + should be compatible with .NET core in the future, so we should go for it as it's the best bet both performance-wise and memory-wise.
This brings another package dependency and changes a bit backend of ConcurrentHashSet
52 lines
1.8 KiB
C#
52 lines
1.8 KiB
C#
/*
|
|
_ _ _ ____ _ _____
|
|
/ \ _ __ ___ | |__ (_)/ ___| | |_ ___ __ _ _ __ ___ | ___|__ _ _ __ _ __ ___
|
|
/ _ \ | '__|/ __|| '_ \ | |\___ \ | __|/ _ \ / _` || '_ ` _ \ | |_ / _` || '__|| '_ ` _ \
|
|
/ ___ \ | | | (__ | | | || | ___) || |_| __/| (_| || | | | | || _|| (_| || | | | | | | |
|
|
/_/ \_\|_| \___||_| |_||_||____/ \__|\___| \__,_||_| |_| |_||_| \__,_||_| |_| |_| |_|
|
|
|
|
Copyright 2015-2017 Łukasz "JustArchi" Domeradzki
|
|
Contact: JustArchi@JustArchi.net
|
|
|
|
Licensed under the Apache License, Version 2.0 (the "License");
|
|
you may not use this file except in compliance with the License.
|
|
You may obtain a copy of the License at
|
|
|
|
http://www.apache.org/licenses/LICENSE-2.0
|
|
|
|
Unless required by applicable law or agreed to in writing, software
|
|
distributed under the License is distributed on an "AS IS" BASIS,
|
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
See the License for the specific language governing permissions and
|
|
limitations under the License.
|
|
|
|
*/
|
|
|
|
using System;
|
|
using System.Collections;
|
|
using System.Collections.Generic;
|
|
using Nito.AsyncEx;
|
|
|
|
namespace ArchiSteamFarm {
|
|
internal sealed class ConcurrentEnumerator<T> : IEnumerator<T> {
|
|
public T Current => Enumerator.Current;
|
|
|
|
private readonly IEnumerator<T> Enumerator;
|
|
private readonly IDisposable Lock;
|
|
|
|
object IEnumerator.Current => Current;
|
|
|
|
internal ConcurrentEnumerator(ICollection<T> collection, AsyncReaderWriterLock rwLock) {
|
|
if ((collection == null) || (rwLock == null)) {
|
|
throw new ArgumentNullException(nameof(collection) + " || " + nameof(rwLock));
|
|
}
|
|
|
|
Lock = rwLock.ReaderLock();
|
|
Enumerator = collection.GetEnumerator();
|
|
}
|
|
|
|
public void Dispose() => Lock.Dispose();
|
|
public bool MoveNext() => Enumerator.MoveNext();
|
|
public void Reset() => Enumerator.Reset();
|
|
}
|
|
} |