Fix race condition in JavaScript TextInterceptorTests

[mbien]

(e.g JsTypedTextInterceptorTest#testDisabledSmartQuotes had a high failure rate)

situation:

• OptionsUtils caches some Preference values and updates those asynchronously via a listener (Preferences event Thread is not EDT even though it has a deceivingly similar name when looked at through the debugger)
• tests expect values to be immediately available after writes to Preferences (and also that clear() isn't delayed)

fix:

• disable async preferences tracking during tests, write values manually via OptionsUtils instead of preferences.
• update all OptionsUtils copies so that the cached values are made volatile since they are shared between threads and fixed lazy init code which used a CAS block as critical section

part of #8183

[mbien]

testDisabledSmartQuotes1: org.netbeans.modules.javascript2.editor.JsDeletedTextInterceptorTest fails too (same cause). We probably have to check usages and put this into a base class.

[neilcsmith-net]

Curious! Are you sure sync() is not just taking long enough to mask the race condition? Is it guaranteed that events will have been fired by its return?

That OptionsUtils class should probably protect the caching map for concurrent access?

Maybe there should be method in OptionsUtils to flush its cache instead? Or even set keys through it. It's not API anyway as far as I can see.

[mbien]

is not just taking long enough to mask the race condition?

yes I had that in mind too since this is the case for all race conditions. If you make the code slow enough they disappear. It is difficult to proof that this is not the case here too - given that sync is surprisingly slow.

I will close this PR later and isolate the three tests to wrap them in a retry script via #8450

[mbien]

That OptionsUtils class should probably protect the caching map for concurrent access?

it stores it in fields, but those should be volatile at the very least, yeah. I won't change anything unless it also fixes the problem completely, since changing the timing could only obfuscate the issue.

Maybe there should be method in OptionsUtils to flush its cache instead? Or even set keys through it. It's not API anyway as far as I can see.

this wouldn't be sufficient. Since nothing knows what is still in the event queue at this point. I observed the clear event arriving after the Preference write but before the write event arrived. The "happens before" promise doesn't exist if the event thread itself can't be flushed.

[mbien]

the fact that this was copied around everywhere:

find . -type f -iname OptionsUtils.java
./webcommon/javascript2.editor/src/org/netbeans/modules/javascript2/editor/options/OptionsUtils.java                                                                                                                               
./php/php.twig/src/org/netbeans/modules/php/twig/editor/ui/options/OptionsUtils.java
./php/php.editor/src/org/netbeans/modules/php/editor/options/OptionsUtils.java
./ide/db.sql.editor/src/org/netbeans/modules/db/sql/editor/OptionsUtils.java

doesn't really motivate me to work on this at all tbh.

[neilcsmith-net]

That OptionsUtils class should probably protect the caching map for concurrent access?

it stores it in fields, but those should be volatile at the very least, yeah.

hmmm, I totally misread where the storage was at a glance! 😆 But yes, possibly volatile ...

Maybe there should be method in OptionsUtils to flush its cache instead? Or even set keys through it. It's not API anyway as far as I can see.

this wouldn't be sufficient. Since nothing knows what is still in the event queue at this point.

I was only thinking of the test requirements, not a general fix. Enough to ensure the setting in the test works synchronously.

Another option might be to add a temporary second preference listener (assuming they're called in the order they're added) and listen for the required event(s) to propagate?

[mbien]

another attempt:

• disabled the async preferences updater during tests, the values are set directly
• made all cached values volatile to fix the OptionsUtils incarnations

(verified with ~400 local test runs)

lazy init via CAS block is also incorrect, but lets see if the tests work first before making more changes.

[mbien]

planing to merge maintenance PRs like this one here soon.

[mbien]

rebasing, planning to merge it once green

[mbien]

reverted a test too much. Will update in a moment.