search

maca88 / AsyncGenerator

Generating async c# code using Roslyn
MIT License
47 stars 16 forks source link

Async locking #130

Open hazzik opened 5 years ago

hazzik commented 5 years ago

@maca88 https://github.com/nhibernate/nhibernate-core/pull/2147#issue-275171711 :

Currently, the async generator is generating separate fields for lock statements that contain an async invocation, which may cause troubles as two threads may simultaneously execute the same code, one for async and one in sync version of the method.

I think the rules should be like following:

  1. If there are several methods with [MethodImpl(Synchronized)] in a class the generator shall generate only one locker fields for all these methods.

  2. (Optional) The locker should pass the current lock object to the async lock. (MethodImplOptions.Synchronized is actually equivalent to lock(GetType())/lock(this))

maca88 commented 5 years ago

  1. I do agree.

  2. The problem with async locking is that all implementations that I am aware of do not support to pass a lock object which would be used to lock a thread. By not having such async locking, it is not possible to achieve blocking different threads from executing the same code, which can lead to various bugs that are hard to debug. Allowing such behavior doesn't seem right to me and that is why my plan was to remove support for MethodImpl attribute and lock statement. Leaving this as optional, maybe, but I don't think this should be set by default.