Skip to content
This repository has been archived by the owner on Feb 24, 2022. It is now read-only.

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

Open
mashirofang opened this issue Jun 26, 2019 · 3 comments

Comments

@mashirofang
Copy link

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?

@lburgazzoli
Copy link
Collaborator

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

@mashirofang
Copy link
Author

Of course ,I'm glad to do that

@mashirofang
Copy link
Author

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

done with a simple lock

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants