Ticket #868 (closed task: fixed)

Opened 8 years ago

Last modified 7 years ago

Verify waitForThreads() code in LegacyPlugin is safe

Reported by: bdezonia Owned by: bdezonia
Priority: critical Milestone: imagej2-b3-headless
Component: Legacy Compatibility Version:
Severity: serious Keywords:
Cc: Blocked By:
Blocking: #1011

Description

A second legacy plugin can be run while a first one is already running. Make sure the separate waitForThreads() calls in LegacyPlugin don't interact badly.

Imagine possible bad sequence:

  • slow plugin started
  • start second plugin which remembers orig threads
  • slow plugin hatches new thread
  • second plugin terminates and see new thread as its own and waits for it to terminate

A couple things:

  • extra waiting might be the only bad side effect here
  • each plugin records the active threads relative to the current thread's thread group. as long as each plugin is in its own threadgroup no problems should arise.

Change History

comment:1 Changed 8 years ago by bdezonia

  • Priority changed from major to critical
  • Milestone changed from imagej-2.0-beta2 to imagej-2.0-beta1

Putting print statements into LegacyPlugin:run() shows that it takes a very LONG time for waitForThreads() to return. Thus users can run other commands upon an image that looks loaded but in fact is not entirely harmonized. This causes some commands to fail.

Also some plugins hatch other plugins which may or may not work (like Compile And Run and others [the complicated version of Line Width...? Edit LUT ... ? these are guesses]).

So we really need to design a thread watching capability to LegacyPlugin that:

  • notices ALL the threads that get created by by a plugin
  • terminates this checking quickly if possible
  • or AT LEAST locks the current Dataset/ImagePlus such that nobody else can change them

This is now a critical item.

comment:2 Changed 8 years ago by bdezonia

CompileAndRun has been debugged and is working. Plugins that call other plugins work fine too.

The remaining tasks for this ticket are:

1) certainly add a method to lock the currently active Display/Dataset while it is being run through LegaycPlugin::run().

2) maybe find ways to detect thread termination quicker so that users aren't waiting on waitForThreads() so much. When loading Boats the plugin terminates a few seconds after the image is first displayed. Any changes the user makes in those three seconds can get lost or can interfere with the IJ1 plugin results.

comment:3 Changed 8 years ago by bdezonia

With a3ba7159826fbb9a60585340613d3cceb0782ef2 waitForThreads() performance greatly improved.

TODO

1) add a method to lock the currently active Display/Dataset while it is being run through LegaycPlugin::run().

2) have waitForThreads() code reviewed by someone else

comment:4 Changed 7 years ago by curtis

  • Blocking 1011 added

comment:5 Changed 7 years ago by curtis

  • Milestone changed from imagej-2.0-beta1 to imagej-2.0-beta2

comment:6 Changed 7 years ago by bdezonia

After some debugging I think this slow wait might apply in limited circumstances.

Opening Blobs takes a long time. Dumping thread info I see a number of Image Fetcher threads that are in a TIMED_WAITING state. I think the threads aren't closing due to some socket waiting or similar. Image Fetcher is nothing defined by IJ1, IJ2, ImgLib, or SCIFIO/BioFormats. It must be Java itself.

If you have a local copy of Blobs and you use the IJ1 version of File Open it loads nearly immediately and has an inverted lut too. We should determine what specific commands give long waiting times.

I've improved waitForPluginThreads() by whitelisting these Image Fetcher threads that are in a TIMED_WAITING state. This immediately improves Open Sample *. I believe this is safe. Changes present in e7ace4f02559cc404d64329d6c310cd8bc912e8f.

comment:7 Changed 7 years ago by bdezonia

There are non-hanging issues with this approach. See #1154.

comment:8 Changed 7 years ago by bdezonia

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

Remaining issues with this ticket now hatched in #1192. Closing.

Note: See TracTickets for help on using tickets.