On Mon, 13 Oct 2025 17:10:20 GMT, Andy Goryachev <[email protected]> wrote:
> I know you do - the links were added for sake of others :-) very kind ;) > The answer to your question is - I don't know. If one can guarantee that > there is only one thread accessing it, we may need to add a comment (though > things might change in the future, and no one reads comments elsewhere in the > code). And if not, then some form of synchronization is mandatory. I ran all system tests to see when/how the active field was read/modified and on which thread. All operations did happen on the main/eventThread, hence single-threaded. Since all of this is about processing events for the eventThread, this makes sense. But to make sure, I did the following research: There are 3 places in NestedRunnableProcessor where the active field is used (read or write) `Object newRunLoop()` The first time this is invoked is when the `HeadlessApplication` starts, when `Application.run` is called. This creates a new Thread, which is the eventThread. The first thing this thread does is invoking `runLoop` (hence, it is executed on the eventThread). Apart from the first time, this is invoked from `Application.enterNestedEventLoop`. which clearly states in its javadoc that the method may only be invoked on the main (event handling) thread, and it even does a `checkEventThread()` for this. `void leaveCurrentLoop()` This is invoked from `Application.leaveNestedEventLoop`, which has the same docs + check as `enterNestedEventLoop` `void stopProcessing()` This is invoked from Application.terminate, which starts with a `checkEventThread` In summary, all operations in the NestedRunnableProcessor or controlled/gated by the com.sun.glass.ui.Application which makes sure that all access is guarded by `checkEventThread` > I prefer not to think though, and make sure we don't plant booby traps in the > code. I don't think I follow this approach. There are many concerns about "JavaFX being slow", so if we can avoid it, I think we should. That doesn't give green light to plant booby traps of course ;) > > BTW, what is the cost of volatile? 0.1 ns? ;-) I'm not an expert, but I believe it is more than 1 ns on "typical desktop systems". Since this is evaluated on every invocation of a Runnable on the AppThread (hence all pulses), plus on every Runnable executed via e.g. Platform.runLater(), this is something that can be invoked very often. If there was even a single cornercase that we could not rule out, or document as "you're on your own here", I would absolutely agree to make it volatile. But as far as I see, there is no legal case where issues can occur that would have been prevented by adding volatile in this case. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1934#discussion_r2428410682
