Mark JAIIOServiceImpl as optional

[dscho]

It is all-too-easy to mess up, say, a Fiji installation and end up with OMERO's version of jai_imageio.jar instead of SCIFIO's. In such a case, the JAIIOServiceImpl will fail to load and because it is not marked optional, it will completely block the SciJava Context from being instantiated.

This happened with the TrackMate-dev update site because the previous imagej-maven-plugin version overwrote SCIFIO's jai_imageio.jar with OMERO's.

Alternatively, the class should use JAI lazily so that it can be instantiated, but fails to perform its job iff the user wants to use it despite having a messed up installation.

[hinerm]

I'm fine marking this optional, but I have two questions:

1. What part of the initialization fails exactly..? There isn't an initialize method in JAIIIOServiceImpl, so it's not exactly clear to me what's going on...

2. Any reason not to rename the jai_imageio.jar artifact in our jai-imageio component as an alternative?

[dscho]

1. What part of the initialization fails exactly..?

It is a NoClassDefFoundError... which stops the complete initialization of the SciJava Context. In other words, it prevents SciJava Context instances from being constructed at all.

2. Any reason not to rename the jai_imageio.jar artifact in our jai-imageio component as an alternative?

That would be okay, but it does not solve the underlying problem that problems preventing this service from starting up escalate to problems preventing the SciJava context from starting up -- even if the user does not want to do any file I/O but just happens to have an incomplete SCIFIO in the class path...

[ctrueden]

I agree that we should rename jai_imageio to avoid future name clashes. I also agree that as a general rule, we should mark services as Optional whenever feasible, to avoid problems like this in the future. @dscho and I also discussed whether to change the behavior of SciJava Common to make Context startup more aggressive about continuing, but I am ambivalent about it. Having that as the default behavior causes other sorts of laster problems which are harder to diagnose (i.e., it doesn't fail fast anymore).

We could add a mode which can be toggled via a system property, and Fiji could toggle it on, but Fiji would then suffer from the latter "latent" type of problems which result from non-fail-fast systems.

I want to emphasize that no matter what we do, with an extensible system like this, it will be possible (and maybe even probable) for third parties to create their own update sites which are malformed such that when enabled, they hose the entire Fiji installation. Of course we should do what we can to minimize that possibility, but it will always be there. And those bugs are ultimately not our responsibility to work around.

[ctrueden]

To elaborate on the use of Optional a bit more: using it in more places gives us a result much like the general non-fail-fast behavior, except that in the case of Optional services it is assumed that we already have coded things in such a way to handle relevant null references. So we gain the benefits of the more aggressive context startup without imposing it on the whole system and thus suffering its downsides.

[dscho]

And those bugs are ultimately not our responsibility to work around.

I am afraid that this will never get my signature. If a Fiji installation is hosed by simply checking a box to follow an update site, it should always be possible to launch Fiji (or ImageJ!), still. And it should always be possible to launch the updater inside the application, uncheck the box and be happy again.

If checking said box prevents Fiji or ImageJ or the Updater from starting up, we will need two modes: one that fails fast, for developers, and one that complains a lot but still tries its best to start things up.

Think about it: at the end of the day, the justification for our salaries comes from the users. Not from the developers using our software.

[ctrueden]

If a Fiji installation is hosed by simply checking a box to follow an update site, it should always be possible to launch Fiji (or ImageJ!), still.

Then the updater infrastructure needs some changes. It must then not be possible to shadow various key JAR files, for instance. And care must be taken to ensure that if e.g. net.imagej.updater.UpdaterUI appears in other JAR files, that that version of the class will not take precedence. Perhaps we should put all core JARs into a separate folder like "core" which other update sites are not allowed to touch, and then have a separate mode of the ImageJ launcher which only includes those classes on the classpath. And also: not allow any other update sites to shadow the launcher executables, of course.

[dscho]

Those changes would still not address the issue that ImageJ would fail to start altogether.

[ctrueden]

Sure they would. My proposal is essentially a "safe mode" for ImageJ that excludes content added by third party update sites. The only way there would be problems is if the JAR files in the "core" folder had version skew. But we would put only the minimal JARs necessary to run the updater in there... not even SCIFIO would need to go in there.

[dscho]

BTW this issue is the same as #18.

And the more sophisticated the fix is, the more likely it is wrong. I think the fundamental problem here is that we are talking about developers and users alike, and we expect the same level of leniency to apply in both contexts. But that's not true! When users start up Fiji, they should never be prevented from using it when some services fail to initialize, as long as they do not require those services for their work!

So we need to figure out a way to make an ImageJ2 context fail upon non-initializable services when running in unit tests and/or inside an IDE, but not fail when running through the ImageJ launcher.

[hinerm]

@ctrueden seems like this issue is not exclusively SCIFIO-specific.. we should discuss at some point.

[ctrueden]

Since the above discussion, I added a scijava.context.strict property that tries harder to spin up the context successfully. In practice, it does not seem to work very well, though more testing is needed. Regardless, the ImageJ launcher should certainly be changed to enable this feature by default, since there is little harm in the attempt. imagej/imagej-launcher#28