Dealing with nested events for a single property in JavaFX

Thu Jun 21 16:30:48 PDT 2012

No I believe the value would still be "true" immediately. And to be clear I'm talking about Event events here, not change notification listeners. So ActionEvent, MouseEvent, etc.

But that does give pause to consider listeners as well. I don't think you can queue those up. So specifically I think this issue is about whether:

node.fireEvent(newEvent)

is layered or queued, without having to be placed on the actual EventQueue (ie: runLater).

Richard

On Jun 21, 2012, at 4:18 PM, Daniel Zwolenski wrote:

> It's not totally clear: I assume you are talking about a queue per property? Or do you mean one big queue for all properties?
> 
> In either case I think we introduce a potential problem with this. Let's say we have: 
> 
>   BooleanProperty one;
>   BooleanProperty two;
>   two.bind(one);
> 
> Then in normal code we do this:
> 
>   one.set(true);
>   two.get(); // value is 'true' as a result of binding 
> 
> But if we do the exact same code in a callback it has a different result: 
> 
>   one.onValueChanged() {
>     one.set(true);
>     two.get(); // value is 'false' as event hasn't been processed yet
>   }
> 
> With a queue per property the problem is limited to only when you are listening on your own value and just maybe we could get away with that (I'm not convinced though, 'feels' dangerous). With one big queue it is really nasty as it would totally change the semantics of code depending on whether it was being called from within a property callback or not. 
> 
> 
> 
> On Fri, Jun 22, 2012 at 8:54 AM, Richard Bair <richard.bair at oracle.com> wrote:
> Right, so what it comes down to is: do we layer events, or queue events? RunLater is wrong because we might insert a pulse between events on the queue so you will render things incorrectly for some brief period of time.
> 
> It seems like handling events sequentially as they occur is the right semantic, rather than the "onion" approach where partway through handling event 1 we issue event 2. The problem here is that event 2 is handled, and then the rest of event 1 is handled, which now has the wrong state (if it was carrying state related to that first event.
> 
> Of course by queuing events it is possible people will end up with handling so many events that the app thread is always busy and not processing the queue -- but then they can get into infinite recursion and all kinds of other issues, so I don't see that as a problem but just the nature of the beast.
> 
> Richard
> 
> On Jun 21, 2012, at 3:47 PM, Jonathan Giles wrote:
> 
> > The point is: can we have a better solution by considering changing the event API such that it queues events, rather than do them immediately as they are fired.
> >
> > -- Jonathan
> >
> > On 22/06/2012 10:46 a.m., Slavko Scekic wrote:
> >> So, the point is: be careful? :)
> >>
> >> On Fri, Jun 22, 2012 at 12:39 AM, Jonathan Giles <jonathan.giles at oracle.com <mailto:jonathan.giles at oracle.com>> wrote:
> >>
> >>    runLater is wrong more often than it is right - so you're right -
> >>    I don't think we should consider that approach (or requiring
> >>    people to have to know to do this)
> >>
> >>    Saying that you should error out when someone tries to update a
> >>    property in the event callback is also wrong, I think. There are
> >>    legitimate use cases, for example as a last ditch form of
> >>    validation: if the value is outside the allowable range, change it
> >>    back to something that is valid. Alternatively, when focus is
> >>    given to a node, request a focus change on another node. I imagine
> >>    our own code would blow up if we took this approach :-)
> >>
> >>    -- Jonathan
> >>
> >>
> >>    On 22/06/2012 10:31 a.m., Daniel Zwolenski wrote:
> >>
> >>        Similar to the ConcurrentModification thing you get with
> >>        Collections right? Could it be handled in a similar way, i.e.
> >>        throw an error if someone tries to update the property they
> >>        are modifying while in the update callback for that property?
> >>        As you say, it's user error, so slapping them on the wrists is ok.
> >>
> >>        The runLater one feels like it could cause its own problems to
> >>        me. The 'single' threadedness of JFX is part of it's design.
> >>        It gives me deterministic behaviour, this feels like it could
> >>        open up small cracks in that. Obviously we wouldn't get
> >>        concurrency/deadlock issues but I suspect we could get things
> >>        in a non-deterministic order as a result of this (e.g. if
> >>        another thread does a runLater somewhere else in the code at
> >>        the same time this runLater is being added). Could end up that
> >>        my property change is overwritten sometimes but not others,
> >>        etc (I'm going more off gut feel here though than concrete
> >>        examples).
> >>
> >>
> >>        On Fri, Jun 22, 2012 at 8:20 AM, Jonathan Giles
> >>        <jonathan.giles at oracle.com <mailto:jonathan.giles at oracle.com>
> >>        <mailto:jonathan.giles at oracle.com
> >>        <mailto:jonathan.giles at oracle.com>>> wrote:
> >>
> >>           Hi all,
> >>
> >>           I'm going to keep this brief as I'm fairly comprehensively
> >>           underwater on the bug count.
> >>
> >>           Recently I've found a pattern of bug that, well, I'm fairly
> >>        sure
> >>           is due to user error, but is not obvious at all (and as such it
> >>           leads to bug reports). In the last week, I've encountered this
> >>           issue twice. The basic issue is that of listening to an
> >>        event (for
> >>           example, a focus change event), and reacting in such a way
> >>        as to
> >>           modify the state of this property (which results in another
> >>        event
> >>           being fired). The end result is non-deterministic (but often
> >>           broken behavior). Interestingly, it has in both my cases
> >>           manifested itself as something that works once, and then fails
> >>           after that forever more.
> >>
> >>           In both cases, after much code digging and debugging (although
> >>           today was made much easier by the same issue last week), I
> >>        believe
> >>           the issue can be worked around simply by wrapping the change to
> >>           the property state (in the event callback) with a
> >>           Platform.runLater(new Runnable() { ...}). This forces the
> >>        second
> >>           property update to happen after the first event has finished
> >>           firing (at some point in the future).
> >>
> >>           However, this isn't a great solution - we're forcing the
> >>        event to
> >>           fire at a later date where the state may have already
> >>        changed. The
> >>           better solution, in my opinion, is to improve the event system
> >>           such that it knows whether an event is already firing, and
> >>        if so
> >>           it will queue up the event to run after the current one has
> >>           finished. I would be interested in hearing whether anyone
> >>        else has
> >>           encountered this kind of bug, or whether they have better
> >>        suggestions.
> >>
> >>           You can see two examples of this bug in the code attached here
> >>           (where the first example is for ComboBox where the value is
> >>           updated in the onAction callback....which is called when value
> >>           changes):
> >>
> >>        http://javafx-jira.kenai.com/browse/RT-22478
> >>        http://javafx-jira.kenai.com/browse/RT-17772
> >>
> >>           -- Jonathan
> >>
> >>
> >>
> >>
> >>
> >
> >
> 
> 