ImageJ quits twice

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

ImageJ quits twice

Curtis Rueden
Hi everyone,

Today I spent some time fixing a new quitting-related bug in ImageJ2: the "ImageJ quits twice" bug. This was an OS-X-specific bug, related to the Mac application menu handling.

When quitting via Apple > Quit (or Cmd+Q), ImageJ would show two different sets of dialogs: the usual ones, and another one that said simply "Quit ImageJ?" with OK/Cancel. Relatedly, ImageJ would also show two About dialogs when you say Apple > About, and take two different actions when you say Apple > Preferences. In a nutshell, this unfortunate behavior was because both ImageJ 1.x and ImageJ2 were trying to handle those actions.

The problem has now been fixed, and made it onto the respective master branches, as follows:

EventHandler: add optional key attribute

Which makes possible:

DefaultAppEventService: specify subscription keys

Which makes the following fully work:

Fix the "Fiji quits twice" bug ;-)

Please note that these fixes have _not_ been uploaded to the Updater yet, but we will do so as soon as we can. There will be a slew of releases by the end of this week.

Regards,
Curtis

P.S. For the very technically inclined (*squints at Dscho*), as well as the archives:

I want to describe one detail while it's still fresh in my mind. In the SciJava Common application framework, if you have multiple services that implement the same service interface, both annotated with "@Plugin(type = Service.class)", then a full-blown application context will instantiate one of each. _However_, if one of them extends the other, the story is different. In the case of AppEventService, there is a DefaultAppEventService in scijava-common, a CoreAppEventService in imagej-plugins-commands and now a LegacyAppEventService in imagej-legacy. Both of the latter two extend the Default one. Hence, when the Context is initializing its services, it creates them in priority order: 1) LegacyAppEventService [high]; 2) CoreAppEventService [normal]; and 3) DefaultAppEventService [low]. Except that when it is time to create the DefaultAppEventService, the ServiceHelper skips creating it because there is already one present in the context. _Two_, actually!

This has a weird ramification on event handling. As long as we had only CoreAppEventService and DefaultAppEventService, there is only one AppEventService instance in the context, so only one receives the App*Events, so all is well. As soon as we added LegacyAppEventService, though, suddenly a full context has _two_ different AppEventService implementations, _both_ of which are receiving the App*Event notifications. So each of them was trying to handle the events, resulting in the double quit behavior.

The fix is described in the GitHub links above. We might need to make _all_ @EventHandler methods have some default non-empty key based on their class and method signature. Otherwise, it will only be possible to override behavior of EventHandler methods that take care to provide such a key.


_______________________________________________
ImageJ-devel mailing list
[hidden email]
http://imagej.net/mailman/listinfo/imagej-devel
Reply | Threaded
Open this post in threaded view
|

Re: ImageJ quits twice

Johannes Schindelin
Hi Curtis,

On Wed, 6 Aug 2014, Curtis Rueden wrote:

> P.S. For the very technically inclined (*squints at Dscho*), as well as the
> archives:

*squints back*

> We might need to make _all_ @EventHandler methods have some default
> non-empty key based on their class and method signature. Otherwise, it
> will only be possible to override behavior of EventHandler methods that
> take care to provide such a key.

I fully agree!

Thanks for the concise and informative explanation!
Dscho

_______________________________________________
ImageJ-devel mailing list
[hidden email]
http://imagej.net/mailman/listinfo/imagej-devel
Reply | Threaded
Open this post in threaded view
|

Re: ImageJ quits twice

Curtis Rueden
Hi Dscho,

> We might need to make _all_ @EventHandler methods have some default
> non-empty key based on their class and method signature. Otherwise, it
> will only be possible to override behavior of EventHandler methods that
> take care to provide such a key.

Mark and I looked into doing this, and _almost_ pushed some changes to that effect. However, we realized that it only makes sense to provide a key in the case of singletons. So, for services, it makes sense, but for e.g. a display viewer that listens for DisplayDeletedEvent, it would wreak havoc to limit event handling to only a single instance. We considered adding some cleverness around SingletonPlugin, or even just Service (i.e.: generate a default key if the @EventHandler is in a class compatible with Service), but even then, you may or may not actually want to limit the behavior, depending on which layer of an abstract class hierarchy the event handling method is in.

So for now, until we have a concrete use case otherwise, we are holding off on assigning any default keys. It should be "opt in" -- i.e., a conscious choice -- to say "limit this event handler to only _one_ instance."

-Curtis


On Thu, Aug 7, 2014 at 3:56 AM, Johannes Schindelin <[hidden email]> wrote:
Hi Curtis,

On Wed, 6 Aug 2014, Curtis Rueden wrote:

> P.S. For the very technically inclined (*squints at Dscho*), as well as the
> archives:

*squints back*

> We might need to make _all_ @EventHandler methods have some default
> non-empty key based on their class and method signature. Otherwise, it
> will only be possible to override behavior of EventHandler methods that
> take care to provide such a key.

I fully agree!

Thanks for the concise and informative explanation!
Dscho


_______________________________________________
ImageJ-devel mailing list
[hidden email]
http://imagej.net/mailman/listinfo/imagej-devel