Caliburn.Micro icon indicating copy to clipboard operation
Caliburn.Micro copied to clipboard

IDeactivate has AsyncEventHandler but IActivate has EventHandler?

Open dbruning opened this issue 3 years ago • 1 comments

Hi & thanks for your work on this great little framework!

I just spent some frustrating time debugging a race condition in my app. I have a parent Screen which has child Screens, and I use ScreenExtensions.ConductWith to ensure that the children are activated/deactivated along with the parent.

My expectation was that a call to "await parentScreen.ActivateAsync()" wouldn't complete until all children were activated too. However, that's not the case - I think because ScreenExtensions.ActivateWith() fires off ActivateAsync() on the children without awaiting the result. By contrast, "await parentScreen.DeactivateAsync()" DOES ensure that all children are deactivated before returning. That's because the Deactivated event handler is an AsyncEventHandler, and the DeactivateWith() method awaits the child DeactivateAsync() calls.

Further, it seems that Conductor.Collection.AllActive and Conductor.Collection.OneActive behave according to my expectation - so it's just Screen.ConductWith() that has the race condition.

Is there a reason that the behaviour differs between Activate and Deactivate? As a workaround, I'm considering subclassing Screen into something like "ConductingScreen", and using that instead of ConductWith(). But I would prefer to it to be fixed in the framework if possible.

dbruning avatar Jun 17 '21 20:06 dbruning

I think you are right, the ActivateWith method uses a normal EventHandler and DeactivateWith uses an AsyncEventHandler. I suppose we should implement the AsyncEventHandler for ActivateWith.

I will have a look at implementing this.

KasperSK avatar Jun 18 '21 06:06 KasperSK