[ELY-1096][JBEAP-10473]: Avoid possible race conditions incrementing the size of Rotating File Audit

[yersan]

Although currentSize was declared as volatile, this field modifier is not enough to prevent race conditions. This patch uses an AtomicLong instead.

Jira issues:
https://issues.jboss.org/browse/ELY-1096
https://issues.jboss.org/browse/JBEAP-10473

[wildfly-ci]

Can one of the admins verify this patch?

[hkalina]

This should not be needed as preWrite() is called inside of synchronized block:
https://github.com/wildfly-security/wildfly-elytron/blob/master/src/main/java/org/wildfly/security/audit/FileAuditEndpoint.java#L108

[yersan]

Thanks for your review @honza889, yes you're right (I guess you wanted to say the write() method). I agree with you, I didn't check who was calling write(), now I think even the volatile modifier was unnecessary there.

[hkalina]

Actually both of them. You are true, feel free to remove the modifier.

[dmlloyd]

This is not a great solution I think, as it relies on synchronization by external parties. Unless the class as a whole does not have a thread-safety contract established, it's better to replace this with an AtomicLong which has an atomic getAndAdd method on it. Same for the other method below.

On the other hand if the class as a whole needs to update multiple fields atomically, then locking is necessary and should be done directly within the class IMO.

[hkalina]

Similar javadoc would be better in superimplementation - in FileAuditEndpoint - to define contract as suggested by @dmlloyd. To define that any implementation of this methods are called in synchronization block surrounding one log message processing.

[yersan]

Thanks @honza889 , @dmlloyd for the help. I have updated the FileAuditEndpoint contract, maybe it makes more sense now.