For some time now we have been aware of rare but persistent application crashes caused by a race in the Skin.OnInit and sometime Skin.OnUnload methods. Several attempts to find the source of the problem turned up nothing conclusive. Finally with the assistance of our support department and the co-operation of a customer who was experiencing the problem semi-regularly we were able to track down the root cause.
Ultimately the problem was caused by exposing a static List<T> as part of the public API. Access to the list was not thread safe and the internal state of the list could become corrupt and crash the app pool.
To fix this issue in DotNetNuke 6 we considered a few options including a substantial change to the API which would avoid multi-threading issues entirely, and exposing a public SyncRoot through which module developers could ensure thread safe access tot he collections. Major API changes were discarded as being too disruptive to modules that use the existing API. The SyncRoot was discarded becasue if even one module developer failed to use it correctly the problem would not be fixed.
The solution we settled on was to internally manage thread safe access to the lists. This approach does require that we change the public interface from the type List<T> to the interface IList<T>.
Here are the changed method signatures:
In DotNetNuke.Application.DotNetNukeContext
public List<SkinEventListener> SkinEventListeners{ get; }
becomes
public IList<SkinEventListener> SkinEventListeners{ get; }
public List<ContainerEventListener> ContainerEventListeners{ get; }
becomes
public IList<ContainerEventListener> ContainerEventListeners{ get; }
If your module is affected you will see an error similar to this:
System.MissingMethodException
Message=Method not found: 'System.Collections.Generic.List`1<DotNetNuke.UI.Skins.EventListeners.SkinEventListener> DotNetNuke.Application.DotNetNukeContext.get_SkinEventListeners()'.
Recompiling affected modules for DotNetNuke 6.0 is easy, it may require changing explicit references from List to IList, modules which only call xxxEventListeners.Add() will only need recompiled without any code changes.
DotNetNuke 6 will manage the complexities of multi-threading on these lists. There is only one code pattern to watch out for. It is important that any code which explicitly calls GetEnumerator on these lists disposes that enumerator promptly. Enumerators have always been IDisposable and should always be disposed promptly, However since failing to dispose an enumerator has little consequence on a normal list it is a common error when directly dealing with an enumerator.