Skip to content

Client.getInstance() and getRawClient() do not perform safe lazy initialization #25

@Marcono1234

Description

@Marcono1234

The static methods Client.getInstance() and getRawClient() do not perform safe lazy initialization:

public static Client getInstance(){
if ( instance == null ){
instance = new Client();
}
return instance;
}

To correct this perform these steps:

  1. Make instance and rawClient volatile
  2. In the access methods, do double checked locking:
    1. Check if instance == null (for increased performance store current value in local var)
    2. If null:
      1. Synchronize
      2. Check again if instance == null (and overwrite local var, see 2.1)
      3. If null: Initialize (and update local var, see 2.1)
    3. Return instance (or local var) value

E.g.:

private static volatile Client instance;

public Client getInstance() {
    Client localInstance = instance;
    if (localInstance == null) {
        // Could also use dedicated lock for `instance` and `rawClient`, but likely not worth it
        synchronized (Client.class) {
            localInstance = instance;
            if (localInstance == null) {
                instance = localInstance = new Client();
            }
        }
    }
    return localInstance;
}

Or alternatively making these methods synchronized would also suffice and the performance decrease (if any) would likely be negligible.
See also "LCK10-J. Use a correct form of the double-checked locking idiom"

For sanity I would also recommend:

  • Implementing a private constructor, currently Client has the public default constructor
  • Making fields coerceInputs and coerceOutputs final and initializing them from the constructor

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions