etcd4j icon indicating copy to clipboard operation
etcd4j copied to clipboard

may it cause a concurrent problem while using ResponsePromise.addListene method?

Open mashirofang opened this issue 5 years ago • 3 comments

hello! I was planning to use it's async way in my program, and I saw the code in ResponsePromise like this

/**
   * Add a promise to do when Response comes in
   *
   * @param listener to add
   */
  public void addListener(IsSimplePromiseResponseHandler<T> listener) {
    if (handlers == null) {
      handlers = new LinkedList<>();
    }

    handlers.add(listener);

    if (response != null || exception != null) {
      listener.onResponse(this);
    }
  }

  /**
   * Handle the promise
   *
   * @param promise to handle
   */
  protected void handlePromise(Promise<T> promise) {
    if (!promise.isSuccess()) {
      this.setException(promise.cause());
    } else {
      this.response = promise.getNow();
      if (handlers != null) {
        for (IsSimplePromiseResponseHandler<T> h : handlers) {
          h.onResponse(this);
        }
      }
    }
  }



Since it use an un-concurrent-safe "LinkedList" and do not has a lock , would I take the risk that my handler would invoke more than once or somewhere throws an concurrent exception?

mashirofang avatar Jun 26 '19 01:06 mashirofang

yep it could, do you mind sending a pr to fix it ?

lburgazzoli avatar Jun 26 '19 08:06 lburgazzoli

Of course ,I'm glad to do that

mashirofang avatar Jun 26 '19 09:06 mashirofang

yep it could, do you mind sending a pr to fix it ?

done with a simple lock

mashirofang avatar Jun 26 '19 13:06 mashirofang