Ticket #1413 (assigned defect)

As Curtis pointed out, we should not do this. The problem is that making the LegacyService a singleton, we slam the door shut for having multiple independent ImageJ contexts including legacy services side by side.

Instead, we should try to figure out a way how we can have multiple legacy services despite the fact that ImageJ 1.* will always need to be a singleton (due to its over-use of singletons in ij.Menus, ij.WindowManager, etc).

One thing we could do as far as the ImagePlus instances are concerned is to exploit the fact that  http://fiji.sc/javadoc/ij/ImagePlus.html#setProperty%28java.lang.String,%20java.lang.Object%29 takes an object as second parameter, so we could attach the legacy service as a special property.

However, when new images are created in the ImageJ 1.* instance, we will still need a way to figure out which LegacyService to add the mapping to; it will most likely need to be something like a map of threads and thread groups to their corresponding LegacyService, and that map needs to be a singleton.

We chatted about this issue further today. When I suggested allowing multiple LegacyService instances, what I had in mind was either:

1. using the same ImageJ1 singleton if possible (and I acknowledge there could be severe problems with that; we would need to do some exploration and testing); or
2. make subsequent LegacyService instances realize they can't really work properly since another legacy service already exists which owns the ImageJ1 singleton.

In the case of (B), the "nonsingletonness" of services is preserved, with the legacy service failing gracefully when multiple IJ2 contexts exist. It's messy, but supporting multiple simultaneous ImageJ1 instances, when ImageJ1 was never designed to be run in that way, seems like a challenge best avoided here. Our main requirement is to keep existing IJ1-only workflows working, rather than to enable new mixed IJ1/IJ2 workflows.

That said, if it is possible to maintain multiple ImageJ1 instances using separate class loaders, I am not against it, as long as it does not become a maintenance nightmare. Hard to say before we try it!

After further chatting, the following strategy emerged:

• move the implementation of the DefaultLegacyService into a different class.
• make a new DefaultLegacyService that
  • creates a greedy class loader (i.e. it prevents any parent class loader from defining new classes, so that, say, ij.IJ is never loaded into the system class loader)
  • instantiates the implementation in said class loader using ServiceHelper#createExactService(classLoader.loadClass("...Impl"))
  • stores that instance in a field of type LegacyService
  • proxies all LegacyService methods to said instance
• add a check to ...Impl#initialize() to verify that it was loaded using that greedy class loader.

The advantages are:

• We can effectively have multiple IJ1 instances, together with the corresponding LegacyServices, opening the gate to have multiple ImageJ contexts including their own LegacyService.
• As far as IJ1 is concerned, it is still a singleton (within the class loader and its children)
• We do not need to change much code in the legacy service.

The disadvantage is that we do not clean up the code that imitates too much of IJ1's "single computer, single user, single desktop, interactive workflow" paradigm.

Of course, the devil lies in the details, as so often when one tries so hard to avoid resolving technical debt. For example, there might be problems with instantiating two classes instantiating the LegacyService interface in the same ImageJ context.

The good news is that I got this to work, basically. It is an ugly hack involving hard-coded class names in the LegacyClassLoader.

Now the bad news: it is not straight-forward to fix the JUnit tests to pass. The problem is that the class ij.ImagePlus is loaded both into the class loader running the tests as well as the class loader owned by the LegacyService. And instances created in one class loader cannot be passed to the other one; the class definitions are incompatible. This example would therefore not work:

ImagePlus image = IJ.openImage(path);
ImageDisplay display = legacyService.getImageMap().lookupDisplay(image);

Since it would violate the law of the least surprise if such a simple and straight-forward code would not work, we decided to actually go forward with the original plan: support *only one* ImageJ 1.x instance, and let subsequent instantiations of the LegacyService fail.

Of course, should somebody need more than one ImageJ context, each with a working Legacy service, in the same JVM, it is still possible: you just need to make sure that the ImageJ classes are not available from the system class loader, and then have each context in their own class loader.

Therefore we still heed the principle to make the simple easy and the complicated possible.

Hi all, I'm running into this in CellProfiler. We have a feature that lets you reload some of your code (to allow users to fix bugs in their plugins without restarting CellProfiler). It reloads RunImageJ which makes a fresh ImageJ context. That makes the LegacyService instantiation fail. I may have my own bug - the first instance may not have been destroyed prior to the second instantiation.

I think I'm going to patch CellProfiler so that its ImageJ singleton instance is created in code that's not reloaded, but I'd like to follow the resolution of this issue. IMHO, a single ImageJ 1.x instance seems like the least of all evils. Perhaps multiple contexts can pollute each other's state via ImageJ 1.x, but that is what you would expect, knowing what ImageJ 1.x is.

Replying to leek:

I may have my own bug - the first instance may not have been destroyed prior to the second instantiation.

That seems likely. We have done some testing, and as long as you call dispose() on the old ImageJ context first, you should then be able to create another new one. That may be the easiest way to solve your issue, since you only need one context at a time.

Another easy fix: just use separate class loaders. As long as those contexts run in different class loaders, the IJ1 class patching won't interfere.