CGContextClearRect() intermittent crash inside of drawingBlockForImage:withFormatName: about fastimagecache HOT 54 CLOSED

I've been seeing that too. That's what led me to #28. Although I know that fixed some bugs, it's didn't completely resolve the problem for me. I've got a build with a ton of logging out with my beta users. I'll let you know if I get any more hints.

from fastimagecache.

We're experiencing this issue in Path as well, and @mallorypaine suspects that the bitmap context is being botched by something before the drawing block does its thing. It's also a little hard to trigger, so this may be a tricky one to fix.

from fastimagecache.

@LucasTizma Do you need any information from me that I could log remotely?

from fastimagecache.

Do you have a project that reproduces the crash? If so, I'd be happy to take a look and I'm confident we'd be able to identify a fix.

from fastimagecache.

I wish. I’ve never reproduced the bug on my devices. One tester out of twenty has seen the bug, across several beta build of my app. He says it happens at different spots in his timeline, so it’s not like the bug is affecting just one image.

Jared Sinclair
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Friday, January 10, 2014 at 12:51 PM, Mallory Paine wrote:

Do you have a project that reproduces the crash? If so, I'd be happy to take a look and I'm confident we'd be able to identify a fix.

—
Reply to this email directly or view it on GitHub (#31 (comment)).

from fastimagecache.

I have a wild guess that our use of NSMapTable may be related. Can you ship your testers a build with FICImageTable.m:140 commented out so that the NSMapTable is never initialized? Let us know if the bug still occurs.

from fastimagecache.

So, comment out the line:

_chunkMapTable = [NSMapTable mapTableWithKeyOptions:NSMapTableStrongMemory valueOptions:NSMapTableWeakMemory];

Is that right?

Jared Sinclair
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Friday, January 10, 2014 at 12:59 PM, Mallory Paine wrote:

FICImageTable.m:140

from fastimagecache.

Correct.

Sent from my iPhone

On Jan 10, 2014, at 10:01 AM, Jared Sinclair [email protected] wrote:

So, comment out the line:

_chunkMapTable = [NSMapTable mapTableWithKeyOptions:NSMapTableStrongMemory valueOptions:NSMapTableWeakMemory];

Is that right?

Jared Sinclair
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Friday, January 10, 2014 at 12:59 PM, Mallory Paine wrote:

FICImageTable.m:140
—
Reply to this email directly or view it on GitHub.

from fastimagecache.

Okay, shipping it now. Will this change have an effect on performance?

from fastimagecache.

I want to thank you all for publishing this library, too. It'll be shipping in Unread when it launches in the next several weeks. I wouldn't have been able to include inline thumbnails without it.

from fastimagecache.

Couple of other questions: what are the specs for the image formats that you're using? I wonder if this bug could be triggered by specific formats.

from fastimagecache.

@jaredsinclair Congrats on launching Unread! Are you still experiencing this issue? Did commenting out that line help? We are seeing this crash in Castro regularly enough too.

from fastimagecache.

@PadraigK Are you running the latest commit from master? How reproducible is your case? I'd like to debug it if possible.

from fastimagecache.

Sorry, we are running the latest commit but it's not reproducible; we just have crash reports from Hockey.

I did notice fix one issue I noticed 7.1b4 (probably unrelated) in our drawing block where we were creating a new CALayer, then using renderInContext to draw it, the app was logging this to the console:

CoreAnimation: warning, deleted thread with uncommitted CATransaction; set CA_DEBUG_TRANSACTIONS=1 in environment to log backtraces.

I fixed this by wrapping our drawing code in a CATransaction, though I don't fully understand what the original cause was. As I said, I doubt this is related, but thought I'd mention it in case it gives you some ideas.

Thanks so much for sharing this work btw — Castro would be a much poorer performing app without it.

I'd love to know if commenting out that one line works, if anyone can confirm that.

from fastimagecache.

I think I have a reproducible case now, but it depends on me copying in a set of ImageTables, etc to Castro after install. I'm not sure that I could easily build a test project to narrow it down, but I can easily enough hit a breakpoint in Xcode: if there's anything you'd like me to investigate let me know.

I can share the ImageTables folder as well as the various image formats we're using if that helps.

from fastimagecache.

So, you replace the image table files while the app has mmap'd those files? I'm not surprised that you'd encounter issues. FIC assumes that it owns the image table files and that nothing besides FIC will modify those files.

from fastimagecache.

@mallorypaine No, I mean that I can reliably recreate the bug by setting up the app with this set of image table files. They are not touched during the course of the app running, except by FIC.

from fastimagecache.

Update on Unread.

Production vs Beta The app has been out for a week, shipped with the latest master commit of FIC. A small percentage of users (1 to 3 percent) are seeing the same crash discovered in beta, listed at the beginning of this thread. I still haven't been able to reproduce the crash on my devices.

Image Sizes One interesting discovery is that the crash has occurred in more than one spot in the production version of Unread. Unread uses FIC for both the large image thumbnails, as well as the small 58x58 option icons in the option menus. Only the large images generated the CGContextClearRect() crash during the beta period. A very small percentage of production users (less than 1 percent) have encountered the CGContextClearRect() crash in the drawing block for the option icons.

Why Use Clear Rect? In reviewing the code and sample code, I have a question so obvious I can't believe I didn't think to ask it yet: why does the sample code include that CGContextClearRect() line at all? I've tried commenting it out and saw no adverse effects. The context used for the drawing blocks isn't re-used, so what is the purpose of clearing the rect? Is it because the context is using a memory-mapped file for the backing store?

from fastimagecache.

Your draw block may not fill the bounds of the context. If that's the case, then you'd see remnants of the previous image if you didn't clear it first.

But CGContextClearRect is not the problem. If you remove that call, the next drawing call that uses the context will crash instead. The problem is that something has happened to the CGContextRef's backing store (i.e. the memory has been unmapped, or perhaps was never mapped correctly).

One question: does this bug occur in iOS6 as well?

from fastimagecache.

I haven't tried it. Unread requires iOS 7.

from fastimagecache.

When it crashes for us, it's with a 56x56 image, never any of the other sizes.

FICImageFormat *format = [[FICImageFormat alloc] init];
format.style = FICImageFormatStyle32BitBGRA;
format.maximumCount = 100;
format.devices = FICImageFormatDevicePhone;
format.family = SUPImageFamilyPodcastArtwork; 
format.name      = SUPImageFormatNameFullAvatar;
format.imageSize = CGSizeMake(56, 56);

from fastimagecache.

Other than the dimensions, are there any other significant format differences?

~ Jared

On Feb 19, 2014, at 8:01 PM, PadraigK [email protected] wrote:

When it crashes for us, it's with a 56x56 image, never any of the other sizes.

FICImageFormat *format = [[FICImageFormat alloc] init];
format.style = FICImageFormatStyle32BitBGRA;
format.maximumCount = 100;
format.devices = FICImageFormatDevicePhone;
format.family = SUPImageFamilyPodcastArtwork;
format.name = SUPImageFormatNameFullAvatar;
format.imageSize = CGSizeMake(56, 56);
—
Reply to this email directly or view it on GitHub.

from fastimagecache.

Between our other images? No, they're all set up the same other than the dimensions.

Many of them are being generated at the same time when we return YES from the "shouldProcessAllFormatsInFamily" method.

from fastimagecache.

For the first time ever I have seen this bug on my own device. Bonus that it was while attached to the debugger:

Hopefully I can find out something new. :-)

from fastimagecache.

Could you try a build with the mrc branch of FIC?

On Friday, March 7, 2014, Jared Sinclair [email protected] wrote:

For the first time ever I have seen this bug on my own device. Bonus that
it was while attached to the debugger:

[image: screen shot 2014-03-07 at 11 20 27 pm]https://f.cloud.github.com/assets/720600/2364512/34c94eea-a679-11e3-88a7-0f1255b3beb1.png

Hopefully I can find out something new. :-)

Reply to this email directly or view it on GitHubhttps://github.com//issues/31#issuecomment-37088726
.

from fastimagecache.

Okay, I tried a build with the mrc branch, too. Here's what I've learned after reduplicating the bug on both the latest master and the mrc branch (using the -fno-objc-arc flag on the FIC compile sources):

Reproducible I still don't know what I did (or didn't do) do make this bug reproducible, but for the last couple hours the bug crashes the app within seconds of scrolling into content areas that require adding new images to FIC. It reproduces with identical results on both mrc and master.

Two Bugs I see a new bug that hasn't happened before (or at least, it hasn't generated a crash report before). In addition to the known bug when clearing the context in the drawing block, I'm also seeing a crash inside FICImageTable (line 299 on the mrc branch):

So far it seems that there's a 50/50 chance the app will crash on setEntityUUIDBytes: or CGContextClearRect().

from fastimagecache.

Here's a better screenshot of the UUID bug:

from fastimagecache.

Here it is crashing on master with CGContextClearRect():

from fastimagecache.

We were also seeing many entityUUIDBytes crashes in Path, although we have yet to see it in the latest build that includes the MRC version of FIC.

from fastimagecache.

CPU & Memory usage always appear normal. I have FIC's logging delegate method implemented, with no error output prior to the crash.

from fastimagecache.

The last thing I did on my phone before these crashes became reproducible was to force quit every other application from the background (just for idle fun while watching tv). Don't know if that would matter or not.

from fastimagecache.

Here's a link to a folder of all the screenshots I took of the debugger, with subfolders separating master from mrc:

https://www.dropbox.com/sh/l742mybh5uiame0/aCrC-brdKb

from fastimagecache.

Follow up for the night:

1. The problem persisted across every app run for several hours.
2. The clear rect and UUID bugs occurred with roughly equal frequency.
3. Rebooting my phone did not affect the issue. It persisted.

After several hours, I tried resetting the FIC cache via - (void)reset. This immediately resolved the issue. After resetting the cache, I can even run the app under heavy image cache use, writing and reading ~200 large thumbnails (584 px * 368px) while scrolling quickly.

from fastimagecache.

Since the reset I mentioned in my previous comment last night, the bug has not returned.

Up until the reset, the app would crash (either with the UUID or clear rect bug) any time it tried to add an additional image to the cache (approximately 180 existing images far below the max declared by the format).

from fastimagecache.

Ok. Interesting, it seems like this is a data corruption issue of some kind. Would be great to save the app data if you see this again. That way, I could reproduce and debug.

Sent from my iPhone

On Mar 8, 2014, at 11:55 AM, Jared Sinclair [email protected] wrote:

Since the reset I mentioned in my previous comment last night, the bug has not returned.

Up until the reset, the app would crash (either with the UUID or clear rect bug) any time it tried to add an additional image to the cache (approximately 180 existing images far below the max declared by the format).

—
Reply to this email directly or view it on GitHub.

from fastimagecache.

I am also seeing this in Crashlytics @mallorypaine @LucasTizma let me know if you want me to add any additional info to pipe up to Crashlytics when we encounter it again

from fastimagecache.

I'm also seeing this.

from fastimagecache.

Just curious, are we seeing this in 7.1 as well?

from fastimagecache.

Yes:

from fastimagecache.

@PadraigK Do you still have the image table files that reproduce the bug? Would love to investigate today.

from fastimagecache.

Here's the full set:
https://dl.dropboxusercontent.com/u/5917026/ImageTables.zip

But I think the problem was in these ones in particular:
co.supertop.imageformatname.searchresult.full.imageTable
co.supertop.imageformatname.searchresult.full.metadata

from fastimagecache.

@PadraigK Would you mind if I debugged your app as well? It'll be significantly quicker for me to be able to repro the issue. I'm worried that if I try to set this up the FIC demo app, I'll spend a bunch of time but still won't be able to repro. My email is mpaine at google's email service.

from fastimagecache.

@PadraigK For the time being, never mind that last request. I think I've been able to reproduce. Will keep you posted.

from fastimagecache.

In @PadraigK's case, we have an imageTable whose # of entries is weird. _entriesPerChunk is 39, but there are 45 total entries in the table. Thus, this table has two chunks of unequal length. Chunk #1 has 39 entries and chunk #2 has 6 entries. When we go to add a 46th image, we end up extending the length of the file to 84 entries (which is also wrong).

After extending the file length, the 46th entry gets written into chunk #2. But chunk #2 is still in our _chunkDictionary, so we attempt to use it. The problem is that chunk #2 was created with just 6 entries. So as we attempt to write into the 7th entry, we are off the end of the memory we mapped, so we crash.

So, couple of notes here:

1. Total length of file should always be a multiple of _entriesPerChunk. How did we end up with a file with 45 total entries but _entriesPerChunk of 39?
2. If we do find ourselves in the unexpected state mentioned by #1, we should recover from it by reloading the existing partial chunk with its new entry count.
3. When we're at 45 entries, and we need to write #46, why are we jumping to 84 entries instead of 39*2 = 78?

from fastimagecache.

OK, pushed fixes for the cases I outline above. I'd appreciate it if you could update your FIC sources for your next builds so we can see whether or not this bug still occurs.

from fastimagecache.

Lastly, I just pushed a commit which will prevent FIC from instantiating an entry that is out-of-bounds within a chunk. This condition should never occur, but now if it does occur, you'll receive an FIC log message of " *** FIC Error: -[FICImageTable _entryDataAtIndex:] failed to get entry for index " if this condition occurs. And you could set a breakpoint on that log line.

Again, this should never occur given the fixes I pushed earlier. But if it does occur, at least your app won't crash.

from fastimagecache.

@mallorypaine Should the image format's max number of entries be set to a multiple of something in particular?

from fastimagecache.

@mallorypaine Thanks so much for looking into this. We'll include the latest FIC in our next release and report back in a few weeks.

I looked back through our commit history to see if we'd done anything weird like changing the image size or max number of entries between releases, but didn't find anything.

from fastimagecache.

I can confirm that latest version fixed the bug for me.

from fastimagecache.

Path also has not seen this bug since your commits.

from fastimagecache.

Also, for what it's worth, the entityUUIDBytes crash seems to be a separate issue potentially related to content protection. At least it definitely was related to complete content protection for Path. @mallorypaine's latest commit should address content protection issues.

from fastimagecache.

Woohoo. I'm going to close this one out. Let me know if you run into issues while running the latest FIC.

from fastimagecache.

@mallorypaine Helluva bug, no? Thanks again for your time.

from fastimagecache.

@mallorypaine @LucasTizma hey guys -- just stumbled upon this thread as we're experiencing this crash in our latest version of Periscope iOS. We're using the latest cocoapod for FIC. Have you seen any other reports of this crash still occurring?

Is it possible that the latest CocoaPod release is behind your current master branch?

Thanks so much for your help.

from fastimagecache.