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 #1934 (closed defect: fixed)

Opened 2013-07-03T08:29:05-05:00

Last modified 2013-07-12T15:06:27-05:00

Potential unwanted dependency between ModuleService and MenuService

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

Description

The KNIME ImageJ2 integration produces a null pointer exception if the ModuleService comes before the MenuService.

With the beta 7 release of ImageJ2 we had to move ModuleService behind MenuService in the constructor call of the scijava ImageJ Context to avoid a null pointer during initialization (onEvent(ModulesAddedEvent) was called before initialize).

@Curtis

What's weird to me is, I am not sure why we don't see this bug when running the end-user ImageJ application...

Maybe because we use the Context(ServiceClass[]) constructor?

Thanks in advance
Michael

Change History

comment:1 Changed 2013-07-12T12:14:43-05:00 by curtis

I was able to reproduce this bug by changing imagej.Main to:

ImageJ ij = new ImageJ(ModuleService.class, MenuService.class);

And the same NPE can be witnessed.

This bug is a symptom of a race condition, introduced by some changes to how services manage their event handling. It used to be that each service would subscribe itself to events when finished initializing. But that logic was generalized, such that all "contextual" objects (including services) now subscribe themselves as soon as their context is assigned. So now, MenuService subscribes itself to e.g. ModuleAddedEvents before it is finished initializing.

I believe I have fixed the problem in scijava-common: services now defer event handler registration until the end of initialization. If a service overrides the initialize() method, it is important for it to call super.initialize(), or else its event handlers will not be registered. Relevant changeset is:  b019a06a.

I will cut a new release of scijava-common soon and update ImageJ2 to use it; will close this bug when that is done, tested and working.

Last edited 2013-07-12T12:16:15-05:00 by curtis

comment:2 Changed 2013-07-12T12:14:59-05:00 by curtis

  • Cc CurtisRueden removed

comment:3 Changed 2013-07-12T14:39:44-05:00 by curtis

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

It really bothered me that calling super.initialize() was now required for any services overriding initialize(). I think that requiring such a thing is very suboptimal and would result in many future programming errors. So I reworked the fix slightly to avoid that need:  2f08d695.

The change does introduce another Service API method, which breaks downstream Service implementations (though anything extending AbstractService is in the clear). But I think the pain is worth it to fix this critical bug.

I've released SciJava Common 1.5.0 which includes this fix, and updated ImageJ2's master branch to use it:  a229f9ad.

comment:4 Changed 2013-07-12T15:06:27-05:00 by curtis

  • Milestone set to imagej2-b8-analysis