[4.1] Futures removeListener: JavaDoc and implementation are inconsistent - duplicate listeners

[mickare]

Netty version: 4.1.0.CR7

Context:

I encountered an inconsistency between the JavaDoc description and implementation of the Future.
I want to add multiple times (e.g. x 1000) the same close listener to a channel but I don't want this listener to be called multiple times.
Also I don't want to remove the listener prior to adding it. I could do this but I don't like the wasteful Java array copy calls.

JavaDoc vs Implementation

The Netty 4.1 JavaDoc on Future says:

Future removeListener(GenericFutureListener<? extends Future<? super V>> listener)

Removes the specified listener from this future. The specified listener is no longer notified when this future is done. If the specified listener is not associated with this future, this method does nothing and returns silently.

But if I add multiple times the same listener to the future, only the first entry is removed. (see implementation)

With the current implementation in mind the JavaDoc should state that only the first occurrence of the listener is removed. Like:

Removes the first occurrence of the specified listener from this future.

More on futures (could be an independent proposal):

Even more the listener is called multiple times. I think futures are part of the observer pattern. That means an observer should be notified only once an event occurs.

That means there should be no duplicate listeners in the future.
E.g.: the listeners array sould be changed to a set, or duplicates avoided.

Or is there a puprose, that I can not see at the moment? Like performance? But if it is performance, then would not multiple calls on the same listener effect the performance? For example:

private final ChannelFutureListener closeListener = f -> cache.invalidate(f.channel);

public void oftenCalled(Channel ch) {
  ch.closeFuture().addListener(closeListener);
  ...
}

With the current implementation closeListener would be called as often as the method oftenCalled.
I had just a discussion that there could be a use-case of a self-incrementing counter, that uses the future to add a future increment. This counter would need multiple entries.
Therefore I propose to add a new method "addListenerIfAbsent", that could resolve the proposal. I'll create a new issue for that.

Best regards,
Michael

[mickare]

Splitted the proposal to new issue: #5352

[Scottmitch]

@mickare - Thanks for bringing this to our attention. The javadocs should be updated. Let me take care.