NOTICE! This is a static HTML version of a legacy ImageJ Trac ticket.

The ImageJ project now uses GitHub Issues for issue tracking.

Please file all new issues there.

Ticket #2013 (closed defect: invalid)

Opened 2013-10-17T14:41:56-05:00

Last modified 2013-10-22T10:16:22-05:00

QuitProgram code needs to be changed

Reported by: bdezonia Owned by: curtis
Priority: major Milestone: imagej2-b8-analysis
Component: SciJava Common Version:
Severity: serious Keywords:
Cc: Blocked By:
Blocking: #1519

Description

The QuitProgram plugin calls context.dispose(). However the plugin itself is using some services (such as the ThreadService to run itself) that are unhappy. They throw exceptions.

The QuitProgram really needs to just publish an event that says WillBeQuitting. Some people can subscribe to it (like the ScriptEditor which may veto the WillBeQuitting event if the user cancels because a current script has not been saved). After every subscriber to the WillBeQuitting event has decided not to veto then something in SciJava actually initiates the quit (after all plugins and threads have run etc.). This will keep things from barfing on quit.

Change History

comment:1 Changed 2013-10-17T14:42:35-05:00 by bdezonia

  • Blocking 1519 added

comment:2 Changed 2013-10-17T15:04:07-05:00 by bdezonia

See this thread (jumping into middle) for some more info:

 http://imagej.net/pipermail/imagej-devel/2013-October/001783.html

And a little more background here:

 https://github.com/scijava/scijava-common/pull/14

comment:3 Changed 2013-10-22T09:51:38-05:00 by curtis

  • Status changed from new to closed
  • Resolution set to invalid

It is not necessary to change QuitProgram in order to avoid exceptions with ThreadService. I have  fixed the DefaultThreadService so that the problem no longer occurs.

However, I do think there is some merit to making quits vetoable. We should consider how the API for that should work. I filed a new ticket #2016 for this feature.

Last edited 2013-10-22T09:51:53-05:00 by curtis

comment:4 Changed 2013-10-22T10:01:57-05:00 by bdezonia

I think this fix is okay but might not be necessary if #2014 is addressed correctly. Without fixing #2014 behavior can be unpredictable for any services with dependencies. In general when the ThreadService is disposing all services that could actually use the ThreadService should already be gone. I guess a running plugin can try to launch a thread after the context is disposed but that seems like an invalid action to protect against.

comment:5 Changed 2013-10-22T10:04:41-05:00 by curtis

I agree that #2014 also should get fixed. However, regardless of whether the disposal order is fixed, it is still possible for something to retain a handle on a ThreadService and then try to run something after context disposal has occurred. So I think the change to DefaultThreadService was required anyway too.

comment:6 Changed 2013-10-22T10:16:22-05:00 by bdezonia

Yes that is what I said too. If it is not an invalid state to use services after disposal then every service should follow DefaultThreadService's pattern.