9

Title says it all. Is there anything wrong with await Task.Run(() => semaphore.WaitOne());? System.Threading.Semaphore isn't thread-affine, so I wouldn't think there would be a problem. I know that the SemaphoreSlim class is available, but I need to do cross-process synchronization, and SemaphoreSlim doesn't do that.

Or can/should I create my own custom type of WaitHandle?

noseratio
  • 59,932
  • 34
  • 208
  • 486
Zane Kaminski
  • 529
  • 4
  • 13
  • 6
    There is technically no problem doing this. Whether you should do this or not depends on many more details not specified in your post. – Michael Gunter Apr 16 '14 at 19:17
  • 3
    The underlying operation is inherently asynchronous, and yet you're having a thread pool thread synchronously wait on the asynchronous operation so that you can asynchronously indicate when you're done. This is generally poor design, and should be avoided if possible. It's a sign that you probably shouldn't be using `Semaphore` in the first place. – Servy Apr 16 '14 at 19:19
  • This indicates either a mistake on your part or a questionable design. I suspect the former - you say you "need to do cross-process synchronization" but this isn't really going to synchronize anything. If the process calling `WaitOne` actually needs to know when the semaphore is signaled, why doesn't it wait synchronously? If it doesn't need to know, why have the semaphore at all? – nobody Apr 16 '14 at 19:23
  • For brevity in the title, I omitted `await`ing the resultant `Task`. I would have something like: `await Task.Run(() => semaphore.WaitOne()) /* Do work */` or `await Task.Run(() => { semaphore.WaitOne(); /* Do work */ })` – Zane Kaminski Apr 16 '14 at 19:34
  • 2
    Define "wrong". Short answer is: yes, for broad definitions of "wrong". It could be that you're using semaphore in the wrong way--based solely on what you've posted it looks more like named event would be better. `WaitOne()` waits indefinitely, what happens if the app tries to exit before the semaphore is signaled? What happens if the semaphore becomes abandoned? You're not handling exceptions. Etc... – Peter Ritchie Apr 16 '14 at 20:20
  • What kind of app is this? Phone? ASP.NET? – usr Apr 16 '14 at 21:12

2 Answers2

8

If you're trying to keep the UI responsive while waiting for the semaphore here, it might make sense, but there's a catch: "Semaphores don't have owners". If you share the semaphore between two processes, and the other process crashes without calling Semaphore.Release(), the ownership over the shared resource will be lost. The remaining process may not be able to acquire it again.

IMO, the Mutex semantic would be more appropriate here, but with Mutex you'd need thread affinity. Perhaps, you can acquire the mutex, access the resource and release it on the same thread:

await Task.Factory.StartNew(() => 
{
    mutex.WaitOne();
    try
    {
        // use the shared resource
    }
    finally
    {
        mutex.ReleaseMutex(); 
    }
}, TaskCreationOptions.LongRunnning);

If that's not possible (e.g., because you need to access the shared resource on the main UI thread), you could use a dedicated thread for the mutex. This can be done with a custom task scheduler, e.g. Stephen Toub's StaTaskScheduler with numberOfThreads:1 (the helper thread doesn't have to be made STA in this case):

using (var scheduler = new StaTaskScheduler(numberOfThreads: 1))
{
    await Task.Factory.StartNew(
        () => mutex.WaitOne(), 
        CancellationToken.None,
        TaskCreationOptions.None,
        scheduler);
    try
    {
        // use the shared resource on the UI thread
    }
    finally
    {
        Task.Factory.StartNew(
            () => mutex.ReleaseMutex(), 
            CancellationToken.None,
            TaskCreationOptions.None,
            scheduler).Wait();
    }
}

Updated, if you're concerned about WinRT (i.e., .NET for Windows Store Apps) or Windows Phone, then Task.Factory.StartNew w/ TaskCreationOptions.LongRunning is still there, you can use it instead of new Thread() with StaTaskScheduler or something like my ThreadWithSerialSyncContext whenever you need a background thread with affinity.

Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • Nice solution, but I don't think it works for me. I can't use a `Mutex` because I need to await some tasks while I have acquired the semaphore, and so thread-affine locks are a no-go. Using `Wait()` or `RunSynchronously()` also is not an option. The `StaTaskScheduler` solution you proposed sounds good, but unfortunately, in the latest iteration of the WinRT APIs, the `Thread` class is nonexistant. Barring a hard reset of the system (which would clear the semaphore anyway), if I wrap the synchronized code in a try/finally, would there be any possibility of an abandoned semaphore? – Zane Kaminski Apr 17 '14 at 13:28
  • @ZaneKaminski, if you carefully balance `WaitOne`/`Release` with `try`/`finally` everywhere, you should be fine with this approach. – noseratio Apr 17 '14 at 22:11
  • @ZaneKaminski, also check my update regarding WinRT. – noseratio Apr 18 '14 at 00:42
  • Great idea. I didn't even think about that – Zane Kaminski Apr 18 '14 at 05:27
0

You can use a SemaphoreSlim(0, 1) WITH your Semaphore , and WaitAsync before waiting for your original semaphore. This would ensure that for each process, only one thread is blocked waiting for the original Semaphore, and the others, while (a)waiting for WaitAsync, is not blocked.

Another solution is to use the exclusive scheduler of ConcurrentExclusiveSchedulerPair.

Jason Lee
  • 502
  • 6
  • 15