Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Throttle callbacks (again) + crash fix#944

Merged
johnhaley81 merged 3 commits intonodegit:masternodegit/nodegit:masterfrom
srajko:throttle-crash-fixsrajko/nodegit:throttle-crash-fixCopy head branch name to clipboard
Apr 9, 2016
Merged

Throttle callbacks (again) + crash fix#944
johnhaley81 merged 3 commits intonodegit:masternodegit/nodegit:masterfrom
srajko:throttle-crash-fixsrajko/nodegit:throttle-crash-fixCopy head branch name to clipboard

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.