patterncsharpMinor
Thread-safe session manager
Viewed 0 times
managersessionthreadsafe
Problem
Expected behavior of the code below:
Things I'm looking for input on are:
The code (runnable in Linqpad with the inclusion of the
```
void Main()
{
var sm = new SessionManager();
Guid id = sm.StartSession();
sm.UseTheSession(id).Dump();
sm.UseTheSession(id).Dump();
sm.EndSession(id);
}
public class SessionManager
{
private ConcurrentDictionary sessionLocks =
new ConcurrentDictionary();
private ConcurrentDictionary sessions =
new ConcurrentDictionary();
public Guid StartSession()
{
Guid id = Guid.NewGuid();
// Takes a sec to create the session.
var session = new ASession(string.Format("Session {0}", id));
Thread.Sleep(1000);
if(!sessions.TryAdd(id, session))
throw new Exception("Astronomically unlikely situation encountered.");
return id;
}
public int UseTheSession(Guid id)
{
lock(this.GetSessionLocker(id))
{
ASession session;
if(sessions.TryGetValue(id, out session))
{
return this.DoSomethingWithSession(s
- There will be one static
SessionManageraccessed by many threads.
- Multiple calls to
SessionManager.UseTheSessionfor a given session ID must be processed on a first come first serve basis (similarly, callingEndSessionshould allow any currently executingUseTheSession's to finish).
- The goal of the individual session locks is that a call to
SessionManager.UseTheSessionby one thread doesn't hold up another thread that is calling the same method with a different session ID.
Things I'm looking for input on are:
- Will this perform as expected.
- Does
sessionsneed to be aConcurrentDictionaryor does a regularDictionarywork fine here since I'm locking any access of the one key.
- What's a good way to handle the memory leak that happens when I keep creating new locks but don't remove them for a given id after I stop using that it?
The code (runnable in Linqpad with the inclusion of the
System.Collections.Concurrent namespace):```
void Main()
{
var sm = new SessionManager();
Guid id = sm.StartSession();
sm.UseTheSession(id).Dump();
sm.UseTheSession(id).Dump();
sm.EndSession(id);
}
public class SessionManager
{
private ConcurrentDictionary sessionLocks =
new ConcurrentDictionary();
private ConcurrentDictionary sessions =
new ConcurrentDictionary();
public Guid StartSession()
{
Guid id = Guid.NewGuid();
// Takes a sec to create the session.
var session = new ASession(string.Format("Session {0}", id));
Thread.Sleep(1000);
if(!sessions.TryAdd(id, session))
throw new Exception("Astronomically unlikely situation encountered.");
return id;
}
public int UseTheSession(Guid id)
{
lock(this.GetSessionLocker(id))
{
ASession session;
if(sessions.TryGetValue(id, out session))
{
return this.DoSomethingWithSession(s
Solution
In this answer I explain in details why one should not throw System.Exception. You should be throwing
As far as thread safety is concerned, I don't write multithreaded code very often so I might be completely wrong, but I since
InvalidOperationException in the case of the astronomically unlikely situation, and probably an ArgumentException in the case of the non-existing session.As far as thread safety is concerned, I don't write multithreaded code very often so I might be completely wrong, but I since
ConcurrentDictionary is a thread-safe IDictionary implementation, sessions.TryGetValue(id, out session) is already a thread-safe call, doesn't need to be wrapped in a lock block.Context
StackExchange Code Review Q#45655, answer score: 5
Revisions (0)
No revisions yet.