ReactiveX / RxJavaFX

RxJava bindings for JavaFX
Apache License 2.0
519 stars 67 forks source link

emitOnChanged() emits values on JavaFx thread #62

Closed morincer closed 6 years ago

morincer commented 6 years ago

When using JavaFxObservable.emitOnChanged outside of the running application (specifically - to unit-test my view models with JUnit) I've experienced the unexpected "Toolkit not initilized exception" on the first emission. The following sample code throws the exception:

    private ListProperty<String> listProperty = new SimpleListProperty<>(FXCollections.observableArrayList());

    @Test
    public void test() {
        JavaFxObservable.emitOnChanged(listProperty)
                //.subscribeOn(Schedulers.io())
                .subscribe(list -> {
                    Observable.fromIterable(list).subscribe(s -> System.out.println(s));
                });

        listProperty.add("list");
    }

The walkaround for the issue is to subscribe on a specific non-JavaFx scheduler (see the commented line).

thomasnield commented 6 years ago

The best workaround I've found is to instantiate a JFXPanel to initialize the JavaFX platform.

See unit test examples here: https://github.com/ReactiveX/RxJavaFX/blob/2.x/src/test/java/io/reactivex/rxjavafx/sources/JavaFxObservableTest.java

pigelvy commented 6 years ago

Indeed, the idea is to initialise the JavaFX thread before using it. The easiest way being the instanciation of a new JFXPanel.

https://gist.github.com/andytill/3835914#file-javafxthreadingrule-java

morincer commented 6 years ago

@thomasnield, for me, the best workaround is subscribing on io thread - that solves the issue :)

Personally, I don't really like the idea of introducing the JavaFx platform into the viewmodel unit tests. As of the JFXPanel - looks like a dirty hack, I would rather use the JfxRunner (you may check it here: https://github.com/sialcasa/mvvmFX/wiki/JfxRunner) or something similar

thomasnield commented 6 years ago

@pigelvy Okay that looks more kosher. Good to know. I'll remember this approach next time.

morincer commented 6 years ago

@thomasnield, just wondering: why emitting the changes on the list is so thread-sensitive?

pigelvy commented 6 years ago

It's true that one might wonder as to why Observable(List|Map|Set) observations has to be performed in the JFX thread?

Even though such classes are in the javafx.collections package, they can very well be used outside of the JFX thread and with no such threading considerations... Don't you think so?

thomasnield commented 6 years ago

JavaFX-specific collections (e.g. ObservableList) have to be manipulated on the FX thread from what I understand, as that is what the Listeners check for.

https://docs.oracle.com/javase/8/javafx/api/javafx/collections/ObservableList.html

pigelvy commented 6 years ago

On one hand, if the ObservableCollection is returned by a JavaFX Node, it will naturally be listened from the JavaFX thread because the JavaFX Node will update it from the JavaFX thread.

On the other hand, if I decide to create an ObservableCollection for me (using for example FXCollections#observableArrayList()), outside of any JavaFX Node/Thread context, then it does not matter which thread is used. Well... it might matter but that is not the point herer :)

At the end of the day, I think that it is up to the object modifying the ObservableCollection to decide which thread to use. A JavaFX Node will trigger its modifications from the JavaFX because that is the way they work. Another component will trigger its modifications from whichever thread it operates on.

The weird thing about these ObservableCollections is that they are in the javafx.collections package instead of the Java Collections Framework.

morincer commented 6 years ago

Observable collections itself are not glued to the UI - yes, they are a part of the JavaFx, but they're a part of the bindings system, not the UI. And thus they have no direct limitations on threads - it's a perfectly valid scenario to, say, load them in a background thread.

The UI thread limitation is applied only to the UI controls, not to the bindings.

thomasnield commented 6 years ago

Okay, so it looks like I followed a precedent and used subscribeOn(JavaFxScheduler.platform() on all the sources. It's possible that is causing the factories to not be thread-agnostic.

https://github.com/ReactiveX/RxJavaFX/blob/2.x/src/main/java/io/reactivex/rxjavafx/sources/ActionEventSource.java#L42

morincer commented 6 years ago

@thomasnield , as I can see in the ObservableListSource you are intensively using the unsubscribeInEventDispatchThread - that might cause the exception as well. I guess there is no particular reason to use this for the ObservableList - as this class is not actually a part of the UI.

thomasnield commented 6 years ago

@morincer I think that makes sense. Let me try to break this PR and if it all looks good, I'll merge and do another release with other proposed issues resolved

thomasnield commented 6 years ago

Resolved.