This is some next-level race condition, so for those interested:
- Concurrent collections are thread-safe in a way that each operation is atomic
- Naturally if you call two atomic operations in a row, the result is no longer atomic, since there could be some changes between the first and the last
- Certain LINQ operations such as OrderBy(), Reverse(), ToArray(), among more, use internal buffer for operation with certain optimization that checks if input is ICollection, if yes, it calls Count and CopyTo(), for OrderBy in this example
- In result, such LINQ call is not guaranteed to be thread-safe, since it assumes those two calls to be atomic, while they're not in reality.
This issue is quite hard to spot in real applications, since it's not that easy to trigger it (you need to call the operation on ICollection and then have another thread modifying it while enumerating). This is probably why we've never had any real problem until I've discovered this madness with @Aareksio in entirely different project.
As a workaround, we'll explicitly convert some ICollection inputs to IEnumerable, in particular around OrderBy(), so the optimization is skipped and the result is not corrupted.
I've added unit tests which ensure this workaround works properly, and you can easily reproduce the problem by removing AsLinqThreadSafeEnumerable() in them.
See https://github.com/dotnet/runtime/discussions/50687 for more insight
I have no clue who thought that ignoring this issue is a good idea, at the very least concurrent collections should have opt-out mechanism from those optimizations, there is no reason for them to not do that.
When excessive amount of "missing amounts", so items in the set was missing on our side, there was a possibility for our logic to classify a good trade as bad one, because we didn't fill in enough holes, as the subtraction in the condition was calculated on each loop rather than once initially.
Since this could only worsen the neutrality score, but never improve it (as the amounts were sorted ascending), there was no abusive possibility due to that, only ignoring otherwise valid trades classifying them as worse than they were in reality.
* New inventory fetching
* use new method everywhere
* Store description in the asset, add protobuf body as a backing field for InventoryDescription, add properties to description
* parse trade offers as json, stub descriptions, fix build
* formatting, misc fixes
* fix pragma comments
* fix passing tradable property
* fix convesion of assets, add compatibility method
* fix fetching tradeoffers
* use 40k as default count per request
* throw an exception instead of silencing the error
* Initial .NET 8
* Make it compile in release mode ignoring warnings for now
* First round of improvements
* Second round of improvements
* Third round of improvements
* Use new throws
* Fix .NET Framework, YAY, thanks madness!
Madness devs are awesome
* Misc
* Misc
* AF_NETLINK might be required for some http calls
No clue why
* Fix service files
Doesn't do what it should
* Update CardsFarmer.cs
* New improvements
* Address feedback
* Misc
* Misc
* Misc refactor
* Misc
* Add warnings about password security
* Warn about weak steam passwords even if they are encrypted
* Apply feedback
* Apply feedback
* Simplify code
* Move return criteria up a bit for increased performance
* Choose more fitting strings for localization
* Extract const value
* Fix incorrect null reference warning
* Switch prefix operator for postfix one
Co-authored-by: Łukasz Domeradzki <JustArchi@JustArchi.net>
* Add tests
* Disable CA1724
The type name Utilities conflicts in whole or in part with the namespace name 'Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities'.
* Tell users why their password is considered weak
* Apply feedback
* Merge resource comments
* Misc.
* Use library for password testing and Run testing in background
* Clean up
* OncSeparate forbidden phrases forfor IPC passwords (once again)
* Additionally check encryption key
* Add comment about {0}
Co-authored-by: Łukasz Domeradzki <JustArchi@JustArchi.net>
* implement split on newlines
* replace test of splitting on newlines
* sugar-sugar syntax
* Revert "sugar-sugar syntax"
This reverts commit ee9b558faf.
* add test to confirm that paragraph character size is less or equal to continuation character size
* Rewrite SendMessage() functions to account for new rate-limits
* Refactor new message splitting logic into SteamChatMessage.cs
This makes it ready for unit tests
* Change the concept into UTF8-oriented logic
* Misc
* Add fix for another unit test
* Update
* Fix failing test
I SPENT HOURS ON THIS
* Misc
* Misc
* Add additional unit tests ensuring this works as designed
* Misc
* Misc
* Add one more unit test
* Rework the logic to account for new findings
* Misc
* Add unit test verifying exception on too long prefix
* Address first @Abrynos concern
Because we can
* Throw also on too long prefix in regards to newlines
* Correct wrong bytesRead calculation
This worked previously only because we actually never had enough of room for escaped chars anyway and skipped over (2 + 2 missing bytes was smaller than 5 required to make a line)
* Add unit test verifying if calculation was done properly
* Address @Ryzhehvost concern
* Handle empty newlines in the message properly
* Misc
No reason to even calculate utf8 bytes for empty lines
* Misc
* Add unit test verifying the reserved escape message bytes count
* Correct calculation of lines by taking into account \r
* Update ArchiSteamFarm/Steam/Bot.cs
Co-authored-by: Sebastian Göls <6608231+Abrynos@users.noreply.github.com>
* @Abrynos next time check if it compiles without warnings
* Update SteamChatMessage.cs
* Apply @Abrynos idea in a different way
* Rewrite bot part to remove unnecessary delegate
* Add @Ryzhehvost test
* Add debug output
* Extend @Ryzhehvost test for prefix
* Misc
* Misc refactor
* Misc
* Misc
* Add logic for limited accounts, correct for unlimited
Thanks @Ryzhehvost
* Misc
Co-authored-by: Sebastian Göls <6608231+Abrynos@users.noreply.github.com>
Closes#2316
The issue we're facing right now comes from the fact of desynchronization of packages between different projects. Since I didn't find any way to "fix" the package versions of our plugins to the main ASF project, we'll instead use centralized Directory.packages.props which specifies appropriate versions
