0

I've just refactored some code that managed a global state cache of values that didn't have locking to use double check locking. Other than moving the initialisation to a single source (the cache was being set in a controller method), the main reason was to make sure only one thread was performing the initialisation at a time.

The lifetime of the cache is for the lifetime of the application, so once it is set, it should be fine.

I've locked around the code that does the web request.

My colleague, who wrote the initial code, said he was concerned about the queued threads waiting for because the initialisation makes a web request, and if the request is failing, the last thread has to wait for all the rest to go through so may have to wait awhile and so the request should be outside of the lock, but my thoughts are that a request is expensive and unreliable, and should be minimised.

As this cache doesn't expire, it's not a big deal, but what are your thoughts? I suggested that if we wanted the correct solution, we could test the locks, using Monitor.TryLock etc for some more sophisticated locking, but it's probably overkill.

  • Do I understand correctly that your colleague is not concerned about the time a thread is waiting for its own web request to complete, but he is concerned about the time a thread waits for another thread to complete the same web request? – Bart van Ingen Schenau Jan 5 '16 at 13:40
3

If the web request is only required to initialize the cache, then you're correct. Throw a lock around the whole thing. Why bother making N requests when you're going to ignore all but the first one that returns?

If the request is required regardless of the cache, then your colleague is correct. Allow all N requests through without constraint. The first one back initializes the cache. Throw a lock around cache initialization but not the request.

Depending on use, you could also eagerly initialize the cache on application start up. I wouldn't bother with data that's seldom accessed. But if it's absolutely necessary, why not just grab it early and be done with it?

-1

If you want to do lazy initialization, you should consider using Lazy<T>. Don't reinvent this wheel.

If you would prefer to do eager initialization, you should consider using Task<T>.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.