Thread-safety issues

There are several thread-safety issues in the code. I tried to address them, but the code is a bit convoluted and I have lost track of things. I think it's best if those issues are addresses by the original developers:

In general, all fields of classes, which are potentially used in multiple threads, should be final. These are for example in Id4meResolver: ROOT, DNSSEC_FLAG, sr, vr.

Well... DNSSEC_FLAG cannot be final because it's changed whenever a response from a DNS server is received. If the resolver is invoked by multiple threads, then between setting the field and evaluating it, another thread could have modified it, hence making the value of the field completely useless.

There are two possible solutions:

• the lookup(..) / lookupWithType(..) methods throw an exception if the DNSSEC flag is not set
• the lookup(..) / lookupWithType(..) methods return an object containing the actual response String plus the flag

In the getUserinfo(..) method, which eventually calls lookupWithType(..), the DNSSEC flag is logged but otherwise ignored; I don't know if that's intended. If so, then throwing an exception is of course not an option.

Another problem is the Id4meIdentityAuthorityData class: instances of it are shared via the Id4meIdentityAuthorityStorage.INSTANCE.storage map. The map itself is thread-safe, however I replaced Hashtable with ConcurrentHashMap (in !1 (merged)), which is also thread-safe but has a much better performance.

The actual problem here lies in the fact that the elements in that map, namely the Id4meIdentityAuthorityData objects are modified in several places during the login process. These modifications can affect other threads that use the same Id4meIdentityAuthorityData instances: those threads may see inconsistent data and/or overwrite changes made by the first thread. It's also possible that modifications are never seen by other threads if they run on other CPU cores, because the Id4meIdentityAuthorityData object could be accessed from the CPU core's L1 cache and never be synchronized with the RAM. This second problem could be solved by marking all the fields in Id4meIdentityAuthorityData as volatile, but that wouldn't address the problem with inconsistent changes from multiple threads.

My suggestion is to make all fields of Id4meIdentityAuthorityData final and then rewrite the code around that change.