Reputation: 190935
OK some background. I have something similar to this:
class ConnectionFactory
{
public IConnection Connect()
{
if (User.IsAuthenticated) {
return InternalConnect(User.Username, null);
}
return null;
}
public IConnection Connect(string username, string password)
{
return InternalConnect(username, password);
}
private IConnection InternalConnect(string username, string password)
{
IConnection connection;
var cacheKey = Session[CacheKeySessionKey] as string;
if (!string.IsNullOrEmpty(cacheKey)) {
connection = HttpCache[cacheKey] as IConnection;
}
if (!IsGoodConnection(connection) {
connection = MakeConnection(username, password); // very costly
cacheKey = Session[CacheKeySessionKey] = // some key
HttpCache[cacheKey] = connection;
}
return connection;
}
private bool IsGoodConnection(IConnection conn)
{
return conn != null && conn.IsConnected;
}
}
I'm currently running into a concurrency problem where that Connect()
is being called multiple times and creating multiple IConnection
s per request. I only need one. It is being injected using an IoC container into various instances. MakeConnnection
is very costly as it spins up a WCF channel.
My question is: How can I lock the InternalConnect
calls per session? I don't think locking per request is the right way to go as multiple requests can happen per user. I certainly don't want to lock for every call, as this will give bad performance.
I think that doing this is a bad idea:
lock(Session.SessionID)
{
// Implementation of InternalConnect
}
Note: The username and password overload is what I call only on login.
Upvotes: 7
Views: 6076
Reputation: 5687
This is a utility class I use, I can't remember how much of it wrote but I think it is based on code by Stephen Cleary.
It handles async (due to the Nito NuGet package), is concurrent (can handle multiple callers) and tidies up the lock afterwards (the finally clause). You just need to give it a unique key and the function to execute.
using Nito.AsyncEx;
using System;
using System.Collections.Concurrent;
using System.Threading.Tasks;
public static class ThingLocker
{
private static readonly ConcurrentDictionary<string, AsyncLock> locks = new ConcurrentDictionary<string, AsyncLock>();
public static async Task ExecuteLockedFunctionAsync(string key, Func<Task> func)
{
AsyncLock mutex = null;
try
{
mutex = locks.GetOrAdd(key, new AsyncLock());
using (await mutex.LockAsync())
{
await func();
}
}
finally
{
if (mutex != null)
{
locks.TryRemove(key, out var removedValue);
}
}
}
}
You'd use it like this;
await ThingLocker.ExecuteLockedFunctionAsync("user id etc.", () => { DoThingHere(); } );
You could pass it the address of an async function instead which would make it look tidier.
Upvotes: 1
Reputation: 2157
Another option is to store an object in each users session directly.
The lock would look like this:
lock (Session["SessionLock"]) {
// DoStuff
}
and you could create the object in the global.asax when each session is started
protected void Session_Start(object sender, EventArgs e)
{
Session["SessionLock"] = new object();
}
doing it this way means the lock object is automatically removed once a session ends.
Upvotes: 0
Reputation: 5320
Here's a suggestion : Have Connection maker object that has the logic of MakeConnection in it and it locks the whole process the usual way.when a session starts store a connection maker in it and call this method in internal connect method .
here's what I mean :
public class ConnectionMaker
{
private object _lock=new object();
public IConnection MakeConnection()
{
lock(_lock)
{
//
}
}
}
and then in your Session_Start you can have :
Session["ConnectionMaker"]=new ConnectionMaker();
and then in your internal connect :
if(! IsGoodConnection(connection))
{
var connectionMaker=Session["ConnectionMaker"] as ConnectionMaker;
connection=connectionMaker.MakeConnection();
....
}
Upvotes: 0
Reputation: 3451
This is just untested code, from the top of my head, but it may work?
// globally declare a map of session id to mutexes
static ConcurrentDictionary<string, object> mutexMap = new ConcurrentDictionary();
// now you can aquire a lock per session as follows
object mutex = mutexMap.GetOrAdd(session.SessionId, key => new object());
lock(mutex)
{
// Do stuff with the connection
}
You would need to find a way to clear old sessions out of the mutexMap
but that shouldn't be too difficult.
Upvotes: 11
Reputation: 8166
I would have ninject create the class as a singleton and then store the connection within the factory class itself.
When you make the call to InternalConnect, check to see if _connection
is null or not. If it is, new up a new IConnect and assign it to _connection
Upvotes: 0