> #3339 NPE caused by null defaultImage in XtextEditorTickUpdater (updateEditorImage)

[mehmet-karaman]

Set fields to volatile which could possible be accessed by multiple threads. Used local variables to store the value and added check with local variable.

[cdietrich]

can you also have a look at the subclasses

[mehmet-karaman]

There is only one subclass (XtendEditorErrorTickUpdater), which has only one field and that is not set to null..

[mehmet-karaman]

Ah .. I've got it.. Classes which implement this interface IXtextEditorCallback :)

[github-actions]

Test Results

  6 457 files   -  4    6 457 suites   - 4   3h 14m 0s ⏱️ - 3m 0s
 43 241 tests ± 0   42 656 ✅  -  1    584 💤 ±0  0 ❌ ±0  1 🔥 +1 
170 028 runs   - 23  167 693 ✅  - 21  2 334 💤  - 3  0 ❌ ±0  1 🔥 +1 

For more details on these errors, see this check.

Results for commit e9d5e0f. ± Comparison against base commit 2e919f4.

♻️ This comment has been updated with latest results.

[mehmet-karaman]

Found one more case, where this could happen.. OverrideIndicatorModelListener uses the editor, which is unset and its not volatile.. I've also used it as local variable to avoid setting to null afterwards..

[cdietrich]

no i mean the xtend one. do we need the local stuff you do there too

[mehmet-karaman]

Oh now I looked again, the changes in the "OverrideIndicatorModelListener" are not necessary, because the field is accessed only in the UI thread.. So no worker thread should execute this code..

The problem happens only if the UI thread calls the beforeDispose, meanwhile a worker thread accesses the fields which are set by the beforeDispose method to null. If you check the stacktrace in the ticket, you will see that the defaultImage is accessed by the worker thread, where the UI thread already set this defaultImage to null..

[mehmet-karaman]

We can keep the changes.. Maybe there is a worker thread which does the same for the next time.. So It shouldn't harm to keep these changes..