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

SDKHooks: Reset global hookid when unhooking in SH. #916

Merged
merged 1 commit into from Oct 28, 2018

Conversation

@KyleSanderson
Copy link
Member

KyleSanderson commented Oct 28, 2018

This is an ancient bug in SDKHooks (I actually mailed PM about it before I knew better) where we don't reset the global hookids when we unhook functions. This results in double-frees and oddness when the last users of these forwards unload; and the same or new user comes into the fold using the forward again.

This fixes #912
This is also untested.

Copy link
Member

psychonic left a comment

Good catch.

@KyleSanderson KyleSanderson merged commit cb8d92e into master Oct 28, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@KyleSanderson KyleSanderson deleted the issue912 branch Oct 28, 2018
@asherkin

This comment has been minimized.

Copy link
Member

asherkin commented Oct 28, 2018

Other than possibly being a bit surprising, is there any reason for SH_REMOVE_HOOK_ID not to do this itself?

@KyleSanderson

This comment has been minimized.

Copy link
Member Author

KyleSanderson commented Oct 28, 2018

Other than possibly being a bit surprising, is there any reason for SH_REMOVE_HOOK_ID not to do this itself?

While internally I think we'd be okay this could subtly break plugins if they're storing hookids elsewhere. This would only be an issue on recompilation; but I think it's too late to change this behaviour now (and I like it better as-is, anyways).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.