prevent crash when disk cache file is corrupted

[leonskywalker]

We have received several crash reports related to TMDiskCache like below:

Application Specific Information:
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** -[NSKeyedUnarchiver initForReadingWithData:]: incomprehensible archive (0x62, 0x70, 0x6c, 0x69, 0x73, 0x74, 0x30, 0x30)'

Last Exception Backtrace:
0 CoreFoundation 0x3377b3e7 0x336b9000 + 795623
1 libobjc.A.dylib 0x3b5fb963 0x3b5f3000 + 35171
2 CoreFoundation 0x3377b307 0x336b9000 + 795399
3 Foundation 0x340139d1 0x33fe2000 + 203217
4 Foundation 0x340749df 0x33fe2000 + 600543
5 XXXX 0x00ac8183 __34-[TMDiskCache objectForKey:block:]_block_invoke (in XXXX) (TMDiskCache.m:416)
6 libdispatch.dylib 0x3ba15793 0x3ba14000 + 6035
7 libdispatch.dylib 0x3ba18b3b 0x3ba14000 + 19259
8 libdispatch.dylib 0x3ba1667d 0x3ba14000 + 9853
9 libdispatch.dylib 0x3ba19613 0x3ba14000 + 22035
10 libdispatch.dylib 0x3ba197d9 0x3ba14000 + 22489
11 libsystem_c.dylib 0x3ba3d7f1 0x3ba37000 + 26609
12 libsystem_c.dylib 0x3ba3d684 0x3ba37000 + 26244

It is because the users' cache file is somehow corrupted and It seems the line crashed object = [NSKeyedUnarchiver unarchiveObjectWithFile:[fileURL path]]; needs a try catch block to prevent crash when this happens.

[segiddins]

Wouldn't a better approach be to figure out what is causing the exception rather than blindly wrapping a call in a try/catch block?

[leonskywalker]

Yeah, we were also investigating why the file is damaged. The situation is really rare but the crash reports did show up in our system. My best guess is the app might encountered a crash when writing to the disk cache previously.

What makes me think it's necessary to add a try catch block is if the file is damaged, the app will always crash when reading from the cache. In my app, the reading happens right after launch, so affected users can no longer use my app. So I think it is better to at least prevent crash while we trying to discover the true cause of the problem.

[segiddins]

If the issue is indeed an incomplete write, it might be better to write out to a temp dir and atomically move the completed archive.

[leonskywalker]

We are using [TMCache setObject:forKey:block:] to store the data , it already writes the file atomically.

[chadmoone]

👍 We're having issues with this as well, and I was about to submit this same PR. We aren't able to replicate directly, but it shows up in the production crash logs. An issue was also filed for this, but closed because it couldn't be reproduced (#32).

1. There could be a number of things that cause the file to be corrupted, and while in a perfect world we can prevent most of them, 💩 happens.
2. The only way to know that the data is corrupted is to attempt unarchiving, which causes a crash.
3. Since TMCache is abstracting this, there is no other way to prevent this crash.

I think it makes sense to protect against it here.

[RickDT]

Same here. 👍

[ssoper]

+1 would love to see this implemented

[HardipAtWpost]

+1 that something should be done about this.

The app can decide what to do w/ that "bad" data. This way we aren't blindly failing silently, and the app still has a way to recover.

This is the #1 crash in our production app right now, and we can't do anything about it.

[irace]

Sorry for the delay – I agree that this is probably a good change to implement and will look into doing so shortly.

[irace]

Thanks for your contribution, this has been released as part of TMCache 1.2.3.