Ticket #2013 (closed defect: invalid)

Opened 6 years ago

Last modified 6 years ago

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 6 years ago by bdezonia

  • Blocking 1519 added

comment:2 Changed 6 years ago 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 6 years ago 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 6 years ago by curtis (previous) (diff)

comment:4 Changed 6 years ago 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 6 years ago 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 6 years ago 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.

Note: See TracTickets for help on using tickets.