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.
* Downgrade OpenTelemetry.Exporter.Prometheus.AspNetCore due to issues with latest version
* Add unit to asf_bot_farming_minutes_remaining
* Upgrade some packages released last night (already tested to work)
* Don't forget about unit suffix
* Add build and runtime information metrics
It is not recommended to include this information as labels in all
metrics. Instead, we add two special metrics with a constant value of
"1" and restrict those static pieces of information to them
* Remove module version from metrics as it does not work
* Apply feedback
* Deduplicate code
* Reference related issue in upstream repo
* Closes#3156
* Misc
* Misc
* Rewrite update mechanism ONCE AGAIN, this time to eradicate FSW
* Make creating debug directory non-fatal again, like it used to be
* Deduplicate code
* Remove dead code
* Print update cleanup just once
* Address remaining feedback, go back to _old and _new
* One more nice improvement
* Initial implementation of plugin updates
* Update PluginsCore.cs
* Update IPluginUpdates.cs
* Update PluginsCore.cs
* Make it work
* Misc
* Revert "Misc"
This reverts commit bccd1bb2b8.
* Proper fix
* Make plugin updates independent of GitHub
* Final touches
* Misc
* Allow plugin creators for more flexibility in picking from GitHub releases
* Misc rename
* Make changelog internal again
This is ASF implementation detail, make body available instead and let people implement changelogs themselves
* Misc
* Add missing localization
* Add a way to disable plugin updates
* Update PluginsCore.cs
* Update PluginsCore.cs
* Misc
* Update IGitHubPluginUpdates.cs
* Update IGitHubPluginUpdates.cs
* Update IGitHubPluginUpdates.cs
* Update IGitHubPluginUpdates.cs
* Make zip selection ignore case
* Update ArchiSteamFarm/Core/Utilities.cs
Co-authored-by: Vita Chumakova <me@ezhevita.dev>
* Misc error notify
* Add commands and finally call it a day
* Misc progress percentages text
* Misc
* Flip DefaultPluginsUpdateMode as per the voting
* Misc
---------
Co-authored-by: Vita Chumakova <me@ezhevita.dev>
* Flash console window on input request
* Use BELL character instead of Beep, fix flash struct, add support for minimizing and flashing with Windows Terminal
* cross-platform minimization, use alert char instead of number, fix struct again
* remove console window
* formatting
* use MainWindowHandle if it's set (fix flashing winterm if ASF is launched in conhost)
* fix build
* remove support for flashing winterm
- On start, we create/load crash file, if previous startup was less than 5 minutes ago, we also increase the counter
- If counter reaches 5, we abort the process, that is, freeze the process by not loading anything other than auto-updates
- In order for user to recover from above, he needs to manually remove ASF.crash file
- If process exits normally without reaching counter of 5 yet, we remove crash file (reset the counter), normal exit includes SIGTERM, SIGINT, clicking [X], !exit, !restart, and anything else that allows ASF to gracefully quit
- If process exits abnormally, that includes SIGKILL, unhandled exceptions, as well as power outages and anything that prevents ASF from graceful quit, we keep crash file around
- Update procedure, as an exception, allows crash file removal even with counter of 5. This should allow crash file recovery for people that crashed before, without a need of manual removal.
> As of IdentityModel 7x, this is a legacy tool that should be replaced with Microsoft.IdentityModel.JsonWebTokens.
> This is a newer, faster version of System.IdentityModel.Tokens.Jwt that has additional functionality
Thanks to @xPaw findings, it seems that access token we get on logon can be used for all functionality we require in ASF. This means we no longer need to fetch the one from points shop in AWH and can safely remove that.
Since access token in AWH is public API, this commit:
- Makes Bot.AccessToken public API.
- Deprecates ArchiWebHandler.CachedAccessToken with intention of removal in the next version. Until then, it resolves to Bot.AccessToken internally so all plugins can keep working during transition period.
- Deprecates Utilities.ReadJwtToken(), probably nobody else than me used it, just switch over to Utilities.TryReadJwtToken(), much better design.
- Reverts ArchiCacheable parts back to stable API, as we no longer need the breaking change done in #3133
After investigation, it turns out that the token actually has correct scope (THANK GOD), it's the fact that Valve started issuing those on much shorter notice than our cache.
Up until now, we played it smartly by assuming cached access token should be valid for at least 6 hours, since every time we visited the page, we got a new token that was valid for 24h since issuing. This however is no longer the case and Valve seems to recycle the same token for every request now, probably until we get close to its expiration. This also means that with unlucky timing, we might be trying to use access token that has expired already even for up to 6 more hours, which is unwanted and causes all kind of issues, with 403 in trade offers being one of them.
I could make stupid solution and cache token for shorter, e.g. 1 minute, but instead I did 200 IQ move and rewrote the functionality in a way to actually parse that token, its validity, and set the cache to be valid for a brief moment before the token actually expires. This way, we're not only more efficient (we can cache the token even for 24h if needed), but we're also invalidating it as soon as it goes out of the scope.
* Initial implementation of announce with diff
* Add missing logic pieces
* Change in logic
* Fix checksums
* Add deduplication logic
* Update SetPart.cs
* Use standalone endpoint for diff
* Use different hashcode impl
* Update AssetForListing.cs
* Misc
* Push all the changes for this to finally work
* Use original index rather than self-calculated
ASFB makes some calculations based on index, it's better for us to have holes rather than hiding skipped items.
* Handle edge case of no assets after deduplication
* Remove dead code
* Address trim warnings
* Misc optimization
