Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate parallel listener calls #380

Open
marcelomorgado opened this issue May 3, 2019 · 4 comments
Open

Investigate parallel listener calls #380

marcelomorgado opened this issue May 3, 2019 · 4 comments
Labels
tech debt Technical debt not involving bugs or features
Milestone

Comments

@marcelomorgado
Copy link
Contributor

marcelomorgado commented May 3, 2019

On the development environment, are being called simultaneously and resulting in unexpected behaviors.
Actually, blocks are being mined so shortly that a listener for a block is being called before of the call from the last block.
For now, a call will be ignored if has another call running.
Comment extracted from: tasit-action > Action.js > #addConfirmationListener:

// Note:
// On the development env (using ganache-cli)
// Blocks are being mined simultaneously and generating a sort of unexpected behaviors like:
// - once listeners called many times
// - sequential blocks giving same confirmation to a transaction
// - false-positive reorg event emission
// - collaborating for tests non-determinism
//
// Tech debt:
// See if there is another way to avoid these problems, if not
// this solution should be improved with a state structure identifying state per event
//
// Question:
// Is it possible that that behavior (listener concurrent calls for the same event) is desirable?

Extracted from: #369 (comment)

@marcelomorgado marcelomorgado added the tech debt Technical debt not involving bugs or features label May 3, 2019
@marcelomorgado marcelomorgado added this to the 0.2.0 release milestone May 3, 2019
@pcowgill
Copy link
Member

pcowgill commented May 3, 2019

For now, a call will be ignored if has another call running.

Is this explicitly enforced in our code, or is this the default behavior?

@marcelomorgado
Copy link
Contributor Author

@pcowgill commented:

Is the non-determinism mostly for tests where the listener is added before or after sending the tx? Or is it a little of both?

We're sure it's not just getting called again with another confirmation for each instantaneous block, right? If so, we could probably disable that in our code and check number of confirmations on each listener call. Or something like that.

@marcelomorgado
Copy link
Contributor Author

For now, a call will be ignored if has another call running.

Is this explicitly enforced in our code, or is this the default behavior?

Is this explicitly enforced in our code:

From Subscription class:

const listener = async (...args) => {
  const eventListener = this.#eventListeners.get(eventName);
  const { isRunning } = eventListener;

  if (isRunning) {
    console.info(`Listener is already running`);
    return;
  }

  this.#decorateEventListener(eventName, { isRunning: true });
  await baseListener(...args);
  this.#decorateEventListener(eventName, { isRunning: false });
};

@pcowgill
Copy link
Member

pcowgill commented May 7, 2019

Got it - thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Technical debt not involving bugs or features
Projects
None yet
Development

No branches or pull requests

2 participants