Meaningless example in LI.3
leventov opened this issue · 0 comments
As malkusch noted here:
https://github.com/leventov/java-concurrency-checklist#safe-local-dcl
The above code could be fixed as follows: void cleanup() { MyClass lazilyInitialized = this.lazilyInitializedField; if (lazilyInitialized != null) { lazilyInitialized.close(); } }
That is surely a fix to the NPE, but is that really a fix? I think there is no guararantee that a thread might see that non volatile field to make that local assignment in the first place, thus the if block might never be entered by another thread.
I think this is a valid issue, the example doesn't make a lot of sense right now.
I'm not sure how to fix it, yet. Perhaps it should be not a cleanup() but some other kind of method. Or instead of pointing to potential NPE issue, the warning about non-volatile lazily initialized fields should point to visiblity issues instead. However, there is the same visibility issue if the field was volatile, too.