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

Throttle callbacks (again) + crash fix #944

Merged
merged 3 commits into from
Apr 9, 2016

Conversation

srajko
Copy link
Collaborator

@srajko srajko commented Mar 9, 2016

This reverts the removal of throttling, and adds two changes that seem to resolve the crash. The likely fix is 1e585ea, which restores some logic I omitted in the refactoring, thinking it should not make a difference... but it looks like it did.

I am a little worried about the state of the raw object making a difference after its wrapper has been destroyed, but maybe I'm just missing the big picture of what's happening here. In any case, the logic matches what we were doing before, and no crash is better than crash.

@srajko
Copy link
Collaborator Author

srajko commented Mar 9, 2016

This will fix #919

@srajko srajko changed the title Throttle callbacks (again) + crash fix [WIP] Throttle callbacks (again) + crash fix Mar 15, 2016
@srajko srajko force-pushed the throttle-crash-fix branch 3 times, most recently from a5fc13c to e988385 Compare March 29, 2016 21:44
@srajko srajko changed the title [WIP] Throttle callbacks (again) + crash fix Throttle callbacks (again) + crash fix Mar 29, 2016
srajko added 2 commits April 7, 2016 14:46
This HandeScope was introduced with throttling changes, and may have contributed to the crash.
@srajko srajko force-pushed the throttle-crash-fix branch from e988385 to 666bb18 Compare April 7, 2016 21:46
This check was removed with throttling changes, and may have contributed to the crash.
@srajko srajko force-pushed the throttle-crash-fix branch from 666bb18 to b0089b0 Compare April 8, 2016 00:09
@johnhaley81
Copy link
Collaborator

We've been running this in GK now with no reports of breakage. 👍 this is gtg. Thanks @srajko!

@johnhaley81 johnhaley81 merged commit 8427b8e into nodegit:master Apr 9, 2016
@johnhaley81 johnhaley81 deleted the throttle-crash-fix branch April 9, 2016 00:37
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