singletons and javafx threading gives deadlock

[Geoff Groos]

Hey guys,

We're developing a GUI'd application with guice.

I recently discovered a horrifying and rather tragic behavior within our software, once in a while it will enter deadlock when running under a couple specific situations.

I've got this deadlock down to a contention between two resources:

• The JavaFX thread --specifically, we've written a method called runImmedlatelyOnFX. the source code can be found here, but its effectively draining the fx job queue.
• A singleton's constructor.

The scenario is:

1. Worker-Thread requests and instance of singleton-class B from guice.
2. Guice acquires some kind of mutex to indicate that its constructing a singleton, and calls the Class-B constructor on Worker-Thread.
   
3. the JavaFX application thread requests an instance of singleton-class B from guice. It's blocked by guice pending the result from Worker-Thread's Class-B-Construtor call.
4. The constructor for singleton-class B calls runImmediatelyOnFX. It's blocked until the JavaFX thread executes its enqueued job

and we have deadlock.

So I don't think it will be too helpful to get into a deep discussion about our usage of JavaFX, but I'm wondering if anybody else has run into this problem, and if there's some miracle solution I'm not aware of.

The two things I can employ to mitigate/solve this problem:

• There is one overt use case for this runImmediately business, and it has to do with loading fxml documents (typically done in constructors). If I make our UI code a little bit more flexible about when it actually gets an instanced view tree (read: in a callback rather than in the ctor), we can reduce the number of ctor's that block on runImmediately() quite significantly.
• I can use use Quantem's enterNestedEventLoop() to give the appearance of running immediately on the javaFX thread without actually blocking the javaFX thread. This will likely defeat the purpose of the method for some use cases, so the old "enqueue the job and wait" strategy will have to remain available for them, but it will likely be sufficient for most of the callers.
  

Is there something I'm missing? Is there some architectural master stroke somebody can fill me in on to deal with this problem quickly and easily?

One of the things I'm looking to do now is start failing integration tests when they notice the potential for this deadlock. My plan for this is to have SyncingUtilitles.runImmediatelyOnFXThread and guice singletons act like resource acquisitions, with each checking to see if the thread they're running on has already acquired the other. If they have, they would log some kind of warning that they may be entering deadlock. logging a warning will fail our cucumber performance tests and our ranorex UI automation tests, which should prevent us from pushing new code with these issues.

I haven't started writing that provision listener, is it technically possible?

Thanks for any help!

-Geoff

[Nate Bauernfeind]

I believe I've avoided this by 1) using eager singletons and 2) separating object creation from service starting. (I.e. Constructors only construct, and then a start/stop method is required for all services that "do stuff".)

--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-guice...@googlegroups.com.
To post to this group, send email to google...@googlegroups.com.
Visit this group at http://groups.google.com/group/google-guice.
To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/2b89be39-c127-45e0-871d-4e46b42812ae%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Christian Gruber]

Agreed.  Generally making your objects cheap to construct and separating initialization from construction using something like guava's ServiceManager for startup/shutdown logic.

To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/CAHNex9_yOGiT05y52944yQQOrJEWEGZPeoNC350MHaZ27fLOyQ%40mail.gmail.com.

[Geoff Groos]

Regarding the use of eager singletons, the theory would be that there's less resource contention at boot-up time, so there's less likely to be this scenario, but thats not provably so. There's nothing to stop somebody from binding a particularly big (dependency-wise) class as an eager singleton and giving rise to the scenario I've outlined.

Regarding the use of a service manager, what you would imply then is that you would register some initialization component with an injected service at construction time, to circumvent this problem?  That would mean your constructors read like

class SomeComponent implements SomethingInitializerWants{

  @Inject
  public SomeComponent(Dep1 one, Dep2 two /*...*/
                       InitializerService initializer){
    this.one = one;
    this.two = two;
   
    initializer.register(this);
  }

  @Override
  public void doMoreInitialization(){
    one.configure(this, 1.234);
    this.madeModel = two.factorySomething(stuff());
  }

  //...
}

Is that correct?

JavaFX itself has this concept built in, I was silly enough to ignore it without investigating exactly why its structured to have both a constructor and an initializer.

Thanks for your help guys,

 -Geoff

[Nate Bauernfeind]

I'm sorry that I do not know anything about JavaFX, so I can't really comment on the code snippet.

However, the purpose of using eager singletons are so that you can find missing dependencies, cyclical dependencies, etc. at the beginning of your application and not somewhere in the middle (like when a worker thread is asking for a service that hasn't been constructed yet). Your specific example could indeed have occurred with or without eager singletons.

It sounds like your usage pattern could result in a worker thread trying to use a service that had not yet had the opportunity to run its initialization code on the fx thread. That might be something you want to look out for.

Good luck!

To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/b6575c52-4929-4cba-bcf3-3d56ac9eab32%40googlegroups.com.

[Tim Boudreau]

When I worked on NetBeans, we used to joke that SwingUtilities.invokeAndWait() should have been named invokeAndDeadlock() (and nobody who used it outside a unit test would get it past a code review).  This is the same case, just in JavaFX.

The bottom line on this has nothing to do with Guice.  GUI toolkits have locks - lots of them, and frequently one big one for mutating on-screen state.  The locks GUI component code acquires can change across revisions of the toolkit.  If you want to block a background thread waiting for something to happen on the dispatch thread, and there is any possibility that your background thread will be holding any locks at all, your code will be deadlock-prone, and even "looks like it works" code may deadlock when run against some future version of the toolkit.

The only way to win that game is not to play.  The invokeAndWait() pattern is dangerous, and there is provably no way to make it safe.

You might be able to get out of your current dilemma by constructing your singleton eagerly on startup before any GUI code runs or the event loop is even initialized.

But really, if you don't want to be chasing heisenbugs for eternity, you need to stop using the invokeAndWait() pattern and find a design that doesn't require it (hint: learn to love callbacks, and do the post-wait work there).

-Tim