Thread safety of LocalNotary

[peteryhwong]

Looking at https://github.com/CrispOSS/abs-api-parent/blob/master/abs-api/src/main/java/abs/api/LocalNotary.java#L21, I was wondering if IdentityHashMap needs to be threadsafe if it is used in conjunction with a ConcurrentMap.

[nobeh]

Sorry for the late reply on this.

Let's assume that thread-safety for the usage of IdentityHashMap must be there. This means that there's a risk of concurrent modification:

private final ConcurrentMap<URI, Object> refs = new ConcurrentHashMap<>(8192);
private final IdentityHashMap<Object, Reference> ids = new IdentityHashMap<>(8192);

1. An insert on refs leads to an insert on ids iff insert on refs is successful; i.e. new actor registration.
2. A delete on refs has no effect on ids. This is a bug and should be fixed.
3. Searching for an actor leads to accessing refs or ids or both.

We need to study (1) for such a risk. An insert on refs succeeds iff the newly created actors does not have a reference URI (key) that already exists in the notary. The key resolution works based on the default implementation at Reference#name(). See Reference#L82. For this to cause a thread-safety risk, there needs to be two actors (objects) registering themselves with the same name. This could be considered a user error/bug? Though it might be argued that the library should be smart enough to prevent/alarm this?

The next step is to try to create a minimal test case to show how IdentityHashMap can fail under concurrent modification usage.

[nobeh]

I tried to come up with a minimal code to try to produce ConcurrentModificationException on IdentityHashMap. Does this look good?

public static void main(String[] args) {
    final IdentityHashMap<Object, Reference> ids = new IdentityHashMap<>();
    Runnable modifier = new Runnable() {
        @Override
        public void run() {
            String thread = Thread.currentThread().getName();
            NumberFormat nf = NumberFormat.getNumberInstance();
            while (true) {
                try {
                    ids.put(new Object(), Reference.from(UUID.randomUUID().toString()));
                    System.out.println(thread + " -> size=" + nf.format(ids.size()));
                } catch (ConcurrentModificationException e) {
                    System.err.println("Concurrent modification: " + thread + " >>> " + e);
                    System.exit(1);
                }
            }
        }
    };
    IntStream.rangeClosed(1, Runtime.getRuntime().availableProcessors())
        .mapToObj(i -> new Thread(modifier, "t-" + i))
        .forEach(t -> t.start());
    try {
        Thread.sleep(10000);
    } catch (Throwable e) {
        System.err.println("Execution stopped: " + e);
    }
    System.exit(0);
}