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

ß71: Set savedKeyboard to nil after releasing it. Fixes #1254 #1256

Merged
merged 1 commit into from Nov 30, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Nov 29, 2012

I think this is all that's needed to fix #1254

Since @tiennou has played more with input sources, could you have a quick look over it?

Seems to me like exactly the same crash and fix as this one so I'd have thought it's right :)

Correctly based against the release branch. Sorry for the incorrect first pull.

Means a check for if(savedKeyboard) will correctly return NO after it's released.
@tiennou
Copy link
Member

@tiennou tiennou commented Nov 29, 2012

I can't really test it, I'm at work (sorry, I wanted to mention that to you guys, I'm now employed ;-)). But looks good, the crash looks like a perfect "usage of no longer allocated memory", and niling the ivar should fix it.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 29, 2012

I wanted to mention that to you guys, I'm now employed ;-)

Congrats! So more time to spend on Quicksilver right? That's what Rob might
say I guess ;-)

I actually haven't tested this, but I can't reproduce the bug. Seems
perfectly reasonable to me :)

On 29 November 2012 09:32, Etienne Samson notifications@github.com wrote:

I can't really test it, I'm at work (sorry, I wanted to mention that to
you guys, I'm now employed ;-)). But looks good, the crash looks like a
perfect "usage of no longer allocated memory", and niling the ivar should
fix it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1256#issuecomment-10840461.

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 29, 2012

Looks fine to me, but I never saw the crash. Maybe @ybian could test it?

@ybian
Copy link
Contributor

@ybian ybian commented Nov 30, 2012

OK. I could have a try today, but this is not reproducible to me either.

from my iPhone

在 Nov 29, 2012,10:50 PM,Rob McBroom notifications@github.com 写道:

Looks fine to me, but I never saw the crash. Maybe @YBain could test it?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/1256#issuecomment-10849547.

@ybian
Copy link
Contributor

@ybian ybian commented Nov 30, 2012

I have been using the fix for one whole day and I haven't met any crash so far.

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 30, 2012

I have been using the fix for one whole day and I haven't met any crash so far.

OK, I think it's a pretty safe change in any case.

skurfer added a commit that referenced this issue Nov 30, 2012
ß71: Set `savedKeyboard` to `nil` after releasing it. Fixes #1254
@skurfer skurfer merged commit d673be2 into quicksilver:release Nov 30, 2012
@skurfer
Copy link
Member

@skurfer skurfer commented Nov 30, 2012

Not merging this into master since it originated from release and would include lots of other things.

skurfer added a commit that referenced this issue Nov 30, 2012
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.

None yet

4 participants