Question

I have the following class (bulk code omitted):

public class SimpleEventBus implements EventBus {
    private final static Set<EventHandler> EMPTY_SET = Collections.emptySet();
    private final static EventHandler[] EMPTY_ARRAY = new EventHandler[0];
    private final static Supplier<Set<EventHandler>> CONCURRENT_SET_SUPPLIER =
            () -> Collections.newSetFromMap(new ConcurrentHashMap<>());

    private final ConcurrentMap<Class<?>, Set<EventHandler>> eventMapping = new ConcurrentHashMap<>();
    private final Class<?> eventClassConstraint;

    public SimpleEventBus() {
        this(Object.class);
    }

    public SimpleEventBus(final Class<?> eventClassConstraint) {
        this.eventClassConstraint = Objects.requireNonNull(eventClassConstraint);
    }

    ...

    @Override
    public void executeEvent(final Object event) {
        if (eventClassConstraint.isAssignableFrom(event.getClass())) {
            getCopyOfEventHandlers(event.getClass()).forEach(eventHandler -> eventHandler.invoke(event));
        }
    }

    ...

    private Stream<EventHandler> getCopyOfEventHandlers(final Class<?> eventClass) {
        Set<EventHandler> eventHandlers = eventMapping.get(Objects.requireNonNull(eventClass));
        return (eventHandlers == null) 
                ? Stream.empty()
                : Arrays.stream(eventHandlers.toArray(EMPTY_ARRAY));
    }

    ...

}

I am wondering if getCopyOfEventHandlers here still returns a thread safe copy of the set. I know it would if I would write it as such:

private EventHandler[] getCopyOfEventHandlers(final Class<?> eventClass) {
    Set<EventHandler> eventHandlers = eventMapping.get(Objects.requireNonNull(eventClass));
    return (eventHandlers == null) 
            ? EMPTY_ARRAY
            : eventHandlers.toArray(EMPTY_ARRAY);
}

But I am wanting to benefit from Java 8 as much as possible, hence I decided to do it like this:

private Stream<EventHandler> getCopyOfEventHandlers(final Class<?> eventClass) {
    Set<EventHandler> eventHandlers = eventMapping.get(Objects.requireNonNull(eventClass));
    return (eventHandlers == null) 
            ? Stream.empty()
            : Arrays.stream(eventHandlers.toArray(EMPTY_ARRAY));
}

Is this still thread-safe as streams use lazy initialization? Does this mean that the resulting array (from eventHandlers.toArray(EMPTY_ARRAY) gets stored in memory (blocked from GC) implicitely?

Was it helpful?

Solution

eventHandlers.toArray(EMPTY_ARRAY) does create an array immediately - only the stream is lazy. So your two versions are essentially identical from a thread safety perspective.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top