Is KeyFactory thread-safe?

Question

There is a service class that needs to generate PublicKey instances from X.509-encoded public key representations. One instance of this class will service multiple threads. Is it correct to do something like this?

public class MyService {
    private final KeyFactory rsaKeyFactory;

    public MyService() throws NoSuchAlgorithmException {
        rsaKeyFactory = KeyFactory.getInstance("RSA");
    }

    public PublicKey generatePublicKey(byte[] publicKeyBytes) throws GeneralSecurityException {
        return rsaKeyFactory.generatePublic(new X509EncodedKeySpec(publicKeyBytes));
    }
}

I.e. is the KeyFactory instance used here is thread-safe? generatePublicKey() method may be called by different threads concurrently.

Javadocs don't seem to mention thread-safety.

Answer 1

No, if the Javadoc makes no mention of thread-saftey then thread safety is not guaranteed ("synchronization is an implementation detail").^[1] Add a synchronized modifier to your generatePublicKey method (or some other form of locking) to make it thread-safe and be sure to add a Javadoc comment noting that it is supposed to be thread-safe.

See also:

• Is there a standard annotation which should be added to the method's Javadoc to denote that a method should be called on a particular thread?

• Characterizing thread safety (emphasis mine)

  How many times have you looked at the Javadoc for a class, and wondered, "Is this class thread-safe?" In the absence of clear documentation, readers may make bad assumptions about a class's thread safety. Perhaps they'll just assume it is thread-safe when it's not (that's really bad!), or maybe they'll assume that it can be made thread-safe by synchronizing on the object before calling one of its methods (which may be correct, or may simply be inefficient, or in the worst case, could provide only the illusion of thread safety). In any case, it's better to be clear in the documentation about how a class behaves when an instance is shared across threads.

  [...]

  A class's thread-safety behavior is an intrinsic part of its specification, and should be part of its documentation. Because there is no declarative way of describing a class's thread-safety behavior (yet), it must be described textually. While Bloch's five-tier system for describing a class's degree of thread safety does not cover all possible cases, it's a very good start. Certainly we'd all be better off if every class included this degree of threading behavior in its Javadoc.

---

But maybe….

It looks like your use might be (that is, as hunter pointed out in the comments, once you have a KeyFactory instance, it might be safe to call KeyFactory#generatePublic from multiple threads).

A bit of source diving, KeyFactory.getInstance(String) looks something like so:^{[2] [3]}

public static KeyFactory getInstance(String algorithm)
        throws NoSuchAlgorithmException {
    return new KeyFactory(algorithm);
}

Which in turns calls:

private KeyFactory(String algorithm) throws NoSuchAlgorithmException {
    this.algorithm = algorithm;
    List<Service> list = GetInstance.getServices("KeyFactory", algorithm);
    serviceIterator = list.iterator();
    // fetch and instantiate initial spi
    if (nextSpi(null) == null) {
        throw new NoSuchAlgorithmException
            (algorithm + " KeyFactory not available");
    }
}

And nextSpi looks like:

private KeyFactorySpi nextSpi(KeyFactorySpi oldSpi) {
    synchronized (lock) {
        // Truncated for brevity
    }
}

And KeyFactory#generatePublic looks something like so:

public final PublicKey generatePublic(KeySpec keySpec)
        throws InvalidKeySpecException {
    if (serviceIterator == null) {
        return spi.engineGeneratePublic(keySpec);
    }
    // Truncated for brevity
}

It does look like the class does some locking in parts and not others, which (I imagine was for a purpose and) means that they took thread-saftey into consideration. It could mean that they had intended for it to be safe to construct and use a factory for the same algorithm on multiple threads but it might also not mean that. You would need to exhaustively check the code paths for race conditions.

That said, please don't build anything assuming a contract other than what's in the Javadoc.

Answer 2

No, you have to use syncronized blocks if you want thread safe.