Crash Fixes #1015
Merged
Crash Fixes #1015
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| if (![source respondsToSelector:@selector(entryCanBeIndexed:)] || [source entryCanBeIndexed:[self info]]) { | ||
| [self saveIndex]; | ||
| } else { | ||
| // NSLog(@"not caching %@", [self name]); |
There was a problem hiding this comment.
an else which does...nothing? :p
|
It looks pretty much like what we talked about :-) |
|
Yeah, pretty much what we said, except we didn't say anything about having to remove empty OK, I've made those tidy ups. |
|
Yeah, well..."leave the code a little bit cleaner than you found it" or something like that :-) |
|
Crashes like crazy for me. :-) No mention of it in the crash report, but if I let the debugger catch it, it’s in |
| @@ -436,7 +436,7 @@ - (void)saveIndex { | |||
|
|
|||
| // Lock the 'contents' mutablearray so that it cannot be changed whilst it's being written to file | |||
| @synchronized(contents) { | |||
| NSArray *writeArray = [[[self contents] arrayByPerformingSelector:@selector(dictionaryRepresentation)] copy]; | |||
| NSArray *writeArray = [contents arrayByPerformingSelector:@selector(dictionaryRepresentation)]; | |||
| [writeArray writeToFile:[[path stringByAppendingPathComponent:key] stringByAppendingPathExtension:@"qsindex"] atomically:YES]; | |||
| [writeArray release]; | |||
There was a problem hiding this comment.
I think this line is the problem. writeArray is an autoreleased object, so it shouldn't be explicitly released, right? Looks like you removed this line over on the original pull request.
There was a problem hiding this comment.
Aha, yeah I think you're right. This'll fix the problems for this pull. Hopefully, the actual changes I've made will fix the problem… in general :)
See the extra commit in this pull that fixes the couple of problems
|
OK, good. It builds now (and doesn’t crash), though I fear for the future of I also see that the catalog doesn’t disappear from the prefs during a rescan. I’m running with it now, so I’ll keep an eye out for issues. |
|
I'm still seeing the catalog disappear at times, but that wasn't introduced by this. Haven't run into any problems since using it. Should we merge? |
|
Yes please? :) |
|
Merged. In the future, we should probably squash commits and open a new pull request if we have a commit that prevents QS from building or running. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
This builds on #986.
See that pull request for the full discussion. @HenningJ might be interested with the solution that I've used (his solution)