Monitor.TryEnter for multiple resources

Question

I tried searching for this but did not find the suggestion best suited for the issue that I am facing.

My issue is that we have list/stack of available resources (Calculation Engines). These resources are used to perform certain calculation.

The request to perform the calculation is triggered from an external process. So when the request for calculation is made, I need to check if any of the available resources are currently not performing other calculations, If so wait for some time and check again.

I was wondering what the best way to implement this is. I have the following code in place, but not sure if it is very safe.

If you have any further suggestions, that will be great:

void Process(int retries = 0) {
    CalcEngineConnection connection = null;
    bool securedConnection = false;
    foreach (var calcEngineConnection in _connections) {
        securedConnection = Monitor.TryEnter(calcEngineConnection);
        if (securedConnection) {
            connection = calcEngineConnection;
            break;
        }
    }
    if (securedConnection) {
        //Dequeue the next request
        var calcEnginePool = _pendingPool.Dequeue();

        //Perform the operation and exit.
        connection.RunCalc(calcEnginePool);
        Monitor.Exit(connection);
    }
    else {
        if (retries < 10)
            retries += 1;
        Thread.Sleep(200);
        Process(retries);
    }
}

Answer 1

I'm not sure that using Monitor is the best approach here anyway, but if you do decide to go that route, I'd refactor the above code to:

bool TryProcessWithRetries(int retries) {
    for (int attempt = 0; attempt < retries; attempt++) {
        if (TryProcess()) {
            return true;
        }
        Thread.Sleep(200);
    }
    // Throw an exception here instead?
    return false;
}

bool TryProcess() {
    foreach (var connection in _connections) {
        if (TryProcess(connection)) {
            return true;
        }
    }
    return false;
}

bool TryProcess(CalcEngineConnection connection) {
    if (!Monitor.TryEnter(connection)) {
        return false;
    }
    try {
        var calcEnginePool = _pendingPool.Dequeue();
        connection.RunCalc(calcEnginePool);
    } finally {
        Monitor.Exit(connection);
    }
    return true;
}

This decomposes the three pieces of logic:

• Retrying several times
• Trying each connection in a collection
• Trying a single connection

It also avoids using recursion for the sake of it, and puts the Monitor.Exit call into a finally block, which it absolutely should be in.

You could replace the middle method implementation with:

return _connections.Any(TryProcess);

... but that may be a little too "clever" for its own good.

Personally I'd be tempted to move TryProcess into CalcEngineConnection itself - that way this code doesn't need to know about whether or not the connection is able to process something - it's up to the object itself. It means you can avoid having publicly visible locks, and also it would be flexible if some resources could (say) process two requests at a time in the future.