0

Our application manages client subscriptions and keeps loaded projects in memory. When the last client disconnects from a project, after a delay, the project is unloaded.

Unloading a project is an async method because it involves remote server communication via the network.

If another client subscribes for the project to be unloaded at the same time when the unload timer is triggered, bad things will happen.

How can I synchronise that?

Calling the awaitable UnloadProject method within the lock block is not allowed. It is said to be dangerous and likely to fail, but actually I don't need the asynchronous invocation at all here. I'd be happy to call the async method synchronous and wait for the result if I can have the lock synchronisation that serialises all client subscription requests. Parallel execution may be nice but it just doesn't work for operations that are mutually exclusive (like a shared resource).

I suspect this problem isn't limited to the Timer we have here, but applies to any situation where async methods need synchronisation around them.

Here's the code, with a "TODO" comment at the position where I see the synchronisation gap:

public void ClientSubscribeProject(string projectUrlName)
{
    lock (clientSubscriptions)
    {
        // Initialise or increment counter
        if (!clientSubscriptions.ContainsKey(projectUrlName))
        {
            clientSubscriptions.Add(projectUrlName, 1);
        }
        else
        {
            clientSubscriptions[projectUrlName]++;
        }

        // Abort pending timer
        if (unloadTimers.ContainsKey(projectUrlName))
        {
            unloadTimers[projectUrlName].Change(Timeout.Infinite, Timeout.Infinite);
            unloadTimers[projectUrlName].Dispose();
            unloadTimers.Remove(projectUrlName);
        }
    }
}

public void ClientUnsubscribeProject(string projectUrlName)
{
    lock (clientSubscriptions)
    {
        // Decrement counter
        clientSubscriptions[projectUrlName]--;

        // Clear 0 counter and start timer
        if (clientSubscriptions[projectUrlName] == 0)
        {
            var timer = new Timer(
                async (state) =>
                {
                    lock (clientSubscriptions)
                    {
                        if (clientSubscriptions.ContainsKey(projectUrlName))
                            return;   // Another client has subscribed in this moment
                    }
                    // TODO: <-- Still a synchronisation gap!
                    await UnloadProject(projectUrlName);
                },
                null,
                300000,   // 5 minutes
                Timeout.Infinite);

            unloadTimers.Add(projectUrlName, timer);
            clientSubscriptions.Remove(projectUrlName);
        }
    }
}
ygoe
  • 18,655
  • 23
  • 113
  • 210
  • What's happening inside UnloadProject? Can you push your check on clientSubscriptions into that method and handle all of you synchronization in there? You'd have to make your Timer callback synchronous though. – HasaniH Sep 26 '16 at 12:55
  • Unfortunately not, that's a more general method that performs lower-level server communications. – ygoe Sep 26 '16 at 13:04
  • You're going to need to lock any access to clientSubscriptions and unloadTimers within UnloadProject so you may need to pull that logic out of that method to separate it from the lower-level server comms – HasaniH Sep 26 '16 at 13:23
  • Can't you get rid of the locks using any of the Concurrent Collections? https://msdn.microsoft.com/en-us/library/dd997305(v=vs.110).aspx – Peter Bons Sep 26 '16 at 17:44
  • Since using await yields control back to another thread a lock statement around an async method will indeed not compile. Maybe you can use `SemaphoreSlim.WaitAsync` as explained in one of the answers of http://stackoverflow.com/questions/7612602/why-cant-i-use-the-await-operator-within-the-body-of-a-lock-statement – Peter Bons Sep 26 '16 at 17:55

0 Answers0