Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some fields of CSV writers are not initialized while headers are written #915

Open
abyrd opened this issue Dec 1, 2023 · 0 comments
Open

Comments

@abyrd
Copy link
Member

abyrd commented Dec 1, 2023

This problem was encountered at #912 (comment).

In Java, strangely it's possible to read the value of an uninitialized final variable in a constructor and this is not considered an error. This would often fail fast for null references but not for primitives - their "final" values can actually change during execution. Java also disallows setting such final fields before calling a superclass constructor (as superclass constructor calls must be the first line in a constructor).

We didn't hit this case before, but were trying to include a user-specified value (the dual accessibility threshold) in a column header when the CSV writer was initialized.

The problem described above is the subject of a change introduced in Java 22: https://openjdk.org/jeps/447 "statements before super()". Once that change lands we should initialize all the fields in the subclass before calling the constructor.

We also noticed that CsvResultWriter calls setDataColumns before setting the task field. Setting the task field first would allow its contents to be available for creating headers, and is just generally safer since the compiler doesn’t check for initialized values. But at worst this one would result in an NPE because it's not a primitive typed field. We didn't want to change it right before a release in case this had other implications, but it might be a good tweak as part of another changeset on initialization order.

On the other hand, maybe we just shouldn't be performing file I/O in a constructor. That doesn't seem like good form. If we removed that step, the initialization order wouldn't matter as much. But still, once Java allows it, we should avoid situations where final fields can be read before initialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant