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

Check for drop to prevent race conditions #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anvaka
Copy link

@anvaka anvaka commented Mar 15, 2017

The inHandler uses setTimeout() function to schedule this.open() call. If client calls drop.destroy() before timeout is triggered, the instance of this.drop will be set to null which results in unhandled error in open()

This change fixes the unhandled error.

The `inHandler` uses `setTimeout()` function to schedule `this.open()` call. If client calls `drop.destroy()` before timeout is triggered, the instance of `this.drop` will be set to `null` which results in unhandled error in `open()`

This change fixes the unhandled error.
@zmdavis
Copy link

zmdavis commented Mar 16, 2017

Thanks for doing this. I was about to open a similar pull request when I noticed this one.

Based on CONTRIBUTING.md you'll need to run npm run build and gulp version:patch and commit the output if we want this to get merged.

Also, I believe it's possible to run into a similar problem in the close method, so you may want to add a similar fix there.

@zmdavis
Copy link

zmdavis commented Mar 16, 2017

Also note that this problem has already been solved in two slightly different ways (#139 and #153) with no movement afaict on getting either of those PRs merged.

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

Successfully merging this pull request may close these issues.

2 participants