Would this be a valid Extension?
-
using locks, from my understanding this should work as long as lock is being called on the same object.
internal static async Task Lock(this T lockObject, Action DoAction) where T : class
{
await Task.Run(() => { lock(lockObject)
{
DoAction();
}});
} -
using locks, from my understanding this should work as long as lock is being called on the same object.
internal static async Task Lock(this T lockObject, Action DoAction) where T : class
{
await Task.Run(() => { lock(lockObject)
{
DoAction();
}});
}From what I can see, the code you've provided isn't going to achieve what you might expect it to achieve. I'm going to break the logic down so you can see where the confusion lies.
- The await operator here does not appear to affect the locking behaviour. It's only waiting for the Task.Run to complete.
- Inside the
Task.Run, we have alockstatement. You can think of this as running on a different thread from the caller. - Each call to this method will create a new task, which will attempt to acquire the lock independently.
The end result is multiple calls to this method could run concurrently, each in its own task, defeating the purpose of the lock. The lock will still work within each individual task, but it won't prevent multiple tasks from running simultaneously. To my mind, a better version of this would be this:
private static readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1);
internal static async Task Lock(this T lockObject, Action DoAction) where T : class
{
try
{
await _semaphore.WaitAsync();
DoAction();
}
finally
{
_semaphore.Release();
}
} -
using locks, from my understanding this should work as long as lock is being called on the same object.
internal static async Task Lock(this T lockObject, Action DoAction) where T : class
{
await Task.Run(() => { lock(lockObject)
{
DoAction();
}});
}From what I can see, it should work. Whilst held, the lock prevents any other thread from obtaining the lock. And the thread that's holding the lock never* yields, so nothing else can be run on the same thread. (* Assuming
DoActiondoesn't do anything odd, such as point to anasync voidmethod.) But wouldn't it be better to use a proper async-friendly lock instead? Building Async Coordination Primitives, Part 6: AsyncLock - .NET Parallel Programming[^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
From what I can see, it should work. Whilst held, the lock prevents any other thread from obtaining the lock. And the thread that's holding the lock never* yields, so nothing else can be run on the same thread. (* Assuming
DoActiondoesn't do anything odd, such as point to anasync voidmethod.) But wouldn't it be better to use a proper async-friendly lock instead? Building Async Coordination Primitives, Part 6: AsyncLock - .NET Parallel Programming[^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
Within the DoAction() it performs read/write operations on a memory pointer
-
From what I can see, it should work. Whilst held, the lock prevents any other thread from obtaining the lock. And the thread that's holding the lock never* yields, so nothing else can be run on the same thread. (* Assuming
DoActiondoesn't do anything odd, such as point to anasync voidmethod.) But wouldn't it be better to use a proper async-friendly lock instead? Building Async Coordination Primitives, Part 6: AsyncLock - .NET Parallel Programming[^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
This is how the lock is being used.
public async Task GetSamples()
{
if (!ContainsAudio())
throw new ArgumentException(nameof(G711.Alaw));
short[] samples = [];
NativePointer pointer = _nativePointer!;
await pointer.Lock(() =>
{
unsafe
{
var sampleCount = pointer.Size >> 1;
samples = new short[sampleCount];
var memoryBlock = pointer.GetMemoryBlock();
var value = (short*)memoryBlock;
for (var i = 0; i < sampleCount; i++)
{
samples[i] = *value++;
}
}
});
return samples;
} -
From what I can see, the code you've provided isn't going to achieve what you might expect it to achieve. I'm going to break the logic down so you can see where the confusion lies.
- The await operator here does not appear to affect the locking behaviour. It's only waiting for the Task.Run to complete.
- Inside the
Task.Run, we have alockstatement. You can think of this as running on a different thread from the caller. - Each call to this method will create a new task, which will attempt to acquire the lock independently.
The end result is multiple calls to this method could run concurrently, each in its own task, defeating the purpose of the lock. The lock will still work within each individual task, but it won't prevent multiple tasks from running simultaneously. To my mind, a better version of this would be this:
private static readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1);
internal static async Task Lock(this T lockObject, Action DoAction) where T : class
{
try
{
await _semaphore.WaitAsync();
DoAction();
}
finally
{
_semaphore.Release();
}
}Thanks for the hint on the using the SemaphoreSlim. This works much better than the original extension I was using.
