ContainerControlledLifetimeManager throws caught exception

Mar 26, 2008 at 1:21 AM
Everytime I register an instance, in debug mode, I'll get a SynchronizationLockException which is internally caught.

It can get really annoying when debugging.

The exception occurs in ContainerControlledLifetimeManager.TryExit due to a call to SetValue.

I'm trying to figure out what synchronization does in this class and why it was needed but can't figure it out.

Any ways to improve the experience so the exception wouldn't be thrown?
Mar 26, 2008 at 1:36 AM
The synchronization is needed to support proper thread safety when doing multi-threaded buildups on a container controlled object. If thread A starts building a container controlled object, then swaps to thread B, which tries to build the same type, without the synchronization you could end up building two instances of the same "singleton" type. Trust me, the synchronization is needed.

The down side is that there are cases (like when registering an instance) where we just call SetValue. In those cases, we don't have the lock. There's no TryExitMonitor method, so the best we can do is attempt to leave the monitor, and catch the error if we weren't actually holding the lock.

Since the exception is actually being handled, the easiest thing to do would be to turn off "first chance exceptions" for SynchronizationLockException. Then the debugger won't stop anymore unless the exception is unhandled.

If there's some magic attribute or something I can sprinkle in there to tell the debugger not to break, I'd love to know about it. Anyone got any ideas?
Mar 26, 2008 at 2:18 AM
I think one of the problems you're having with synchronization depends on how the ILifetimePolicy is defined.

If you were to change the interface slightly for something like:

public interface ILifetimePolicy
{
  object FindOrCreateValue(Func<object> factory);
  void SetValue(object newValue);
  void RemoveValue();
}

Then you give FindOrCreateValue(...) a sufficient atomic scope to deal with synchronization.

Then the LifeTimeStrategy could call FIndOrCreateValue instead of calling seperately GetValue/SetValue

Anyways... I'll look more into in it again as I tend to find that this synchronization + InUse flag start smelling a bit...

Thoughts?
Mar 26, 2008 at 5:53 AM
The InUse flag is there to prevent a bug that one of our product managers did - he reused the same lifetime manager instance to register three different instances, and hilarity ensued. Since the error he got (Entry point not found) didn't obviously point to the issue, the flag was added to cause an understandable exception as early as possible.

I don't see how FindOrCreateValue would work in the presence of the current strategy chain execution. If you've got an idea (or a working prototype) I'd love to see it.
Mar 26, 2008 at 6:57 AM
I looked at this before posting and my brain told me I had this part figured out... Look like he was wrong ;)

I think that if you implement ContainerControlledLifetimeManager using ReaderWriterLock, it would solve the problem.

And I just figured out what bothered me (smell): By trying to make ContainerControlledLifetimeManager thread-safe, you assumed the caller contract to be that GetValue() would always be called before SetValue() which is obviously not the case.

You also assumed that if GetValue was called and value was null, that SetValue would necessarily be called. That builds a lot of coupling between LifetimeManager and LifetimeStrategy.

On a side note, that's my 3rd flag regarding the new strategy chain revamp where we "lose" context between the pre and post buildup steps. Given the old chain, FindOrCreate would have worked:
object BuildUp(...)
{
...
if (existing == null)
{
  existing = policy.FindOrCreate(() => base.BuildUp(...));
}
...
}
Dec 28, 2008 at 6:00 AM
Edited Mar 24, 2009 at 2:02 AM
Whilst using Unity I had the same issue of "in debug mode, I'll get a SynchronizationLockException which is internally caught" so I thought it might be useful to post a full solution that other people could use.

> You also assumed that if GetValue was called and value was null, that SetValue would necessarily be called.

If SetValue is called without GetValue being called then a subtly different but simpler solution would be to set a boolean variable after entering the monitor so that upon exiting this value can be checked before exiting:

public abstract class SynchronizedLifetimeManager : LifetimeManager, IRequiresRecovery
    {
        private object lockObj = new object();
        private volatile bool hasMonitorEntered;
        /// <summary>
        /// Retrieve a value from the backing store associated with this Lifetime policy.
        /// </summary>
        /// <returns>the object desired, or null if no such object is currently stored.</returns>
        /// <remarks>Calls to this method acquire a lock which is released only if a non-null value
        /// has been set for the lifetime manager.</remarks>
        public override object GetValue()
        {
            Monitor.Enter(lockObj);
            hasMonitorEntered = true;//inside of synchronised section

            object result = SynchronizedGetValue();
            if (result != null)
            {
                Monitor.Exit(lockObj);
            }
            return result;
        }

        /// <summary>
        /// Performs the actual retrieval of a value from the backing store associated
        /// with this Lifetime policy.
        /// </summary>
        /// <returns>the object desired, or null if no such object is currently stored.</returns>
        /// <remarks>This method is invoked by <see cref="SynchronizedLifetimeManager.GetValue"/>
        /// after it has acquired its lock.</remarks>
        protected abstract object SynchronizedGetValue();

        /// <summary>
        /// Stores the given value into backing store for retrieval later.
        /// </summary>
        /// <param name="newValue">The object being stored.</param>
        /// <remarks>Setting a value will attempt to release the lock acquired by
        /// <see cref="SynchronizedLifetimeManager.GetValue"/>.</remarks>
        public override void SetValue(object newValue)
        {
            SynchronizedSetValue(newValue);
            TryExit();
        }

        /// <summary>
        /// Performs the actual storage of the given value into backing store for retrieval later.
        /// </summary>
        /// <param name="newValue">The object being stored.</param>
        /// <remarks>This method is invoked by <see cref="SynchronizedLifetimeManager.SetValue"/>
        /// before releasing its lock.</remarks>
        protected abstract void SynchronizedSetValue(object newValue);

        /// <summary>
        /// Remove the given object from backing store.
        /// </summary>
        public override void RemoveValue()
        {
        }

        /// <summary>
        /// A method that does whatever is needed to clean up
        /// as part of cleaning up after an exception.
        /// </summary>
        /// <remarks>
        /// Don't do anything that could throw in this method,
        /// it will cause later recover operations to get skipped
        /// and play real havok with the stack trace.
        /// </remarks>
        public void Recover()
        {
            TryExit();
        }

        private void TryExit()
        {
            if (hasMonitorEntered)//inside of synchronised section if true, therefore thread safe
            {
                hasMonitorEntered = false;
                Monitor.Exit(lockObj);
            }
        }
    }

Then you just have to create an exact replica of ContainerControlledLifetimeManager which inherits from the new SynchronizedLifetimeManager and still implements IDisposible.

Finally, just call: "containerInstance.RegisterInstance<T>(instance, new ContainerControlledLifetimeManager());" with the new ContainerControlledLifetimeManager.

(Note: I have updated the above code in after ctavares' comments below so that the hasMonitorEntered field is volatile [http://msdn.microsoft.com/en-us/library/x13ttww7.aspx])
Dec 28, 2008 at 7:34 AM
That's an interesting approach. You'd probably need to mark the bool as volatile to get it to work correctly 100% of the time.

Although, if the goal is just to avoid the break in the debugger, the easiest thing to do is turn off the "Just My Code" checkbox in the debugger options. Then it won't break on exceptions that are thrown through user code, only on genuinely unhandled exceptions.
Mar 23, 2009 at 11:45 AM
Hi Richard,

Thanks for your solution. I think it is never a good idea to (almost) force a throw of an exception. Using a flag is much nicer and more performant (exceptions are a performance killer).
Apr 13, 2009 at 9:37 AM
that SynchronizationLockException won't occur when Microsoft.Practices.Unity.dll from
Microsoft Enterprise Library 4.1 - October 2008\Bin is used.