Replace async Redis calls with sync ones
Closed this issue · 2 comments
This is food for thought and invitation to discussion.
The library uses a mix of sync and async calls to Redis.
It's perfectly fine to use async calls in transactions. It's perfectly fine to use async calls with ContinueWith.
However, there are situations, where the same code is use both in transactions, and sequentially, but those calls are used in synchronous methods and are not awaited.
This way, it is [theoretically] possible that the non-awaited task will be disposed before it is completed.
This call, for example:
https://github.com/marcoCasamento/Hangfire.Redis.StackExchange/blob/master/Hangfire.Redis.StackExchange/RedisFetchedJob.cs#L69-L79
should be synchronous in non-transactional mode.
Agree. The code you pointed out should use ListRightPush. That obviously reflect downstream to the calls in RemoveFromFetchedList.
Do you spot other point where this happen ? I believe it's hard to reproduce a problem, but is however possible, especially in some stress scenarios.
Yes, I saw a few other places, just need to look through all the calls. I agree it's not very likely, but would be good to safeguard against this.
I'll work on a PR.