HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Is this code thread-safe - Singleton Implementation using Concurrent Dictionary

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisdictionarysingletonthreadusingsafecodeimplementationconcurrent

Problem

class Connection 
{
    private string param1;
    private string param2;
    private static readonly ConcurrentDictionary, Connection> 
        connections = new ConcurrentDictionary, Connection>();
    private Connection()
    {
    //Prevent instantiation
    }

    private Connection(string param1, string param2)
    {
        this.param1 = param1;
        this.param2 = param2;
    }

    public static Connection getInstance(string param1, string param2)
    { 
        Connection conn = activeConnections.GetOrAdd(new Tuple 
            param1,param2), new Connection (param1, param2));
        return conn;
    }
}

Solution

Never create two connection objects with the same parameters. If one exists, use it.

If you really need to guarantee this, then I think you will need to use locking instead of ConcurrentDictionary.

If it's okay to create duplicate Connections (that will never be used) in rare circumstances, then you can use an overload of GetOrAdd() that takes a lambda that creates the Connection:

return activeConnections.GetOrAdd(
    Tuple.Create(param1,param2), _ => new Connection (param1, param2));


With your current code, every time you call getInstance(), a new Connection is created and then most of the time thrown away.

Code Snippets

return activeConnections.GetOrAdd(
    Tuple.Create(param1,param2), _ => new Connection (param1, param2));

Context

StackExchange Code Review Q#27507, answer score: 2

Revisions (0)

No revisions yet.