Skip to content

Non-atomic increment of volatile fields #191

@marschall

Description

@marschall

There are two non-atomic increments of volatile fields in com.microsoft.sqlserver.jdbc.SocketFinder#noOfThreadsThatNotified and com.microsoft.sqlserver.jdbc.TDSWriter#packetNum. In Java the pre- and post-increment operators on volatile fields are not atomic. In Java volatile fields indicate that state is intended to be changed by multiple threads.

Since com.microsoft.sqlserver.jdbc.SocketFinder#noOfThreadsThatNotified is only used in updateResult and guarded by socketFinderlock I would remove volatile there.

com.microsoft.sqlserver.jdbc.TDSWriter#packetNum is a bit less obvious. The warning could simply be fixed by using either java.util.concurrent.atomic.AtomicInteger or java.util.concurrent.atomic.AtomicIntegerFieldUpdater, the latter has a bit lower overhead. On the other hand without knowing any details about the threading model it's hard to know if that is enough or even necessary. All the other fields changed in methods where packetNum is accessed seem to be non-volatile and the changes don't seem to be atomic. This suggests to me that those methods are not intended to be thread-safe and as a result removing volatile from packetNum would be my recommendation here as well.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions