Potential not thread-safe behaviour in SynchronizedLifetimeManager about abstractions HOT 10 CLOSED

Yes, the whole SynchronizedGetValue() is kind of fishy. I still have very vague idea how it works and why is it required. If you have suggestion how this could be improved I would love to hear it.

from abstractions.

I did some digging around this issue and looks like the whole design rests on assumption that null is not a valid resolved value. So, theoretically it should never return 'null' if value had been resolved.

from abstractions.

Closing for now, feel free to reopen if you've got any ideas how to improve the implementation.

from abstractions.

@ENikS
We have such problem on our production environment. Sometimes our application stucks on Monitor.Enter.
What would You say to implement (change SynchronizedLifetimeManager) some kind of workaround:

public override object GetValue()
{    
  TimeSpan someBigEnoughTimeout = ...
  if(Monitor.TryEnter(this.lockObj, someBigEnoughTimeout))    
  {
    object obj2 = this.SynchronizedGetValue();    
    if (obj2 != null)    
    {
      Monitor.Exit(this.lockObj);    
    }    
    return obj2;
  }
  else
  {
     //do recovery as we know we had stucked
     LogError(); //to make it possible to analyze how often the problem occurs
     this.Recover();
     throw new Exception();
  }
}

Do You see any threats?

from abstractions.

from abstractions.

I've added throwing exception.
Timeout could be 1 minute. To be sure that we have stucked and minimize false positives eg. when we have very busy cpu.

from abstractions.

This will be rather big, breaking change

from abstractions.

You could build locally and try it out.

To use this feature you need to set global timeout like this:

SynchronizedLifetimeManager.ResolveTimeout = 30000; // 30 sec

It will fail with regular ResolutionFailedException and inner exception TimeOutException

If it works as expected I'll cut the release

from abstractions.

We are getting deadlocks on this issue above. If the getvalue locks, whose job is it to setvalue? Seems very bad to leave a lock open like this.
Also, the resolvetimeout isnt a valid fix either unfortunately. This just tries to hide the bug under a pile of poop.
If the container isnt being resolved in any reasonable amount of time, then the application is not usable.

from abstractions.

In latest version there is a way to set timeout for the lock.

from abstractions.