ManagedUI issue: A callback was made on a garbage collected delegate

Nov 17, 2015 at 11:40 AM
I use ManagedUI and have an exception at the end of the installation process:
Managed Debugging Assistant 'CallbackOnCollectedDelegate' has detected a problem in '...WpfSetup\bin\Release\WpfSetup.exe'.

Additional information: A callback was made on a garbage collected delegate of type 'WixSharp.Msi!WindowsInstaller.MsiInstallUIHandler::Invoke'. This may cause application crashes, corruption and data loss. When passing delegates to unmanaged code, they must be kept alive by the managed application until it is guaranteed that they will never be called.
In WixSharp.Msi\MsiSession.cs I see this:
             uiHandler = new MsiInstallUIHandler(OnExternalUI); //must be kept alive until the end of the MsiInstallProduct call
where OnExternalUI is a private function.
Check this topic also: http://stackoverflow.com/questions/7302045/callback-delegates-being-collected
You are creating a delegate object here and passing it to the unmanaged code. Problem is, the garbage collector cannot track references held by native code. The next garbage collection will find no references to the object and collect it.
Also it's working fine if I use Debug version of WixSharp.Msi (because of it's not optimized).
Coordinator
Nov 17, 2015 at 10:53 PM
Great thank you for reporting. This is an interesting one.

>OnExternalUI is a private function.
Actually the fact that it is private is irrelevant as GC doesn't discriminate on the base of the visibility type. And it is not OnExternalUI but uiHandler who get's collected. But it is not where problem is anyway.

You probably noticed my second comment:
finally
{
    ...
    //It is important to reference uiHandler here to keep it alive till the end. 
    //The debug build is more forgiving and referencing uiHandler is not essential as the code is not optimized
    if (uiHandler != null)
        uiHandler = null;
}
This work around was done to keep the uiHandler reference alive until the end of the method call. Exactly in the spirit of the stack overflow post you provided. However... To my surprise in the finally scenario the trick is invalidated. Somehow finally screws JIT and the condition for uiHandler is never executed.

Interestingly enough it is not because of the compiler optimization. Reflector clearly shows that the cal wasn't removed:
Image
But the debugger never visits the line with the condition, meaning that it is JIT who does the damage. It's worth to note that moving the condition below the finally clause restores the power of the work around even though the finally clause scope is not changed in this case:
finally
{
    ...
}

if (uiHandler != null)
    uiHandler = null;
Anyway it is enough to put any meaningful invoke inside of the condition to fool JIT.
if (uiHandler != null)
{
    Environment.SetEnvironmentVariable("ReasonForThis", "IHadToDoSomethingThatJITWouldNotOptimise");
    uiHandler = null;
}
The fix has been committed and will be available with the next release (in a day or so).

Thank you for your feedback.