Closed Robot2037 closed 5 years ago
@Robot2037 Seems crashed when map empty
keys
and values
, can you reproduce in your development environment? if you can, please enable Zombie
in scheme to catch released object.
And for temporary fix, you can disable the weak memory cache by [SDWebImageManager sharedManager].imageCache.config.shouldUseWeakMemoryCache = NO;
.
By the way, you don't need to call clearMemory
by yourself when enter to background. imageCache
uses NSCache
to store image, it would clear when enter background automatically.
@zhongwuzw There are 32 crashes reported during last 4 days in Crashlytics, which started to appear after new app release, which uses SDWebImage 4.4.2. However I'm not able to reproduce issue myself in Xcode.
Thanks for the info, I will remove clearMemory
call when entering background, since is not needed. Anyway I call this function also in didReceiveMemoryWarning
, which could cause crash as well.
@Robot2037 Emm, SD
also handles the situation of memory warning
. Theoretically even though you don't call it by yourself, the crash would also appear, because we call it in SD
. And before we can find the issues, I suggest you to use [SDWebImageManager sharedManager].imageCache.config.shouldUseWeakMemoryCache = NO;
to disable weak
memory, and only use NSCache
to manage images in memory.
Could you mind post some integrity symbolic crash logs?
@zhongwuzw Ok, I din't even know that. It should be stated somewhere in the documentation that clearMemory
function is called automatically and you don't need to call it yourself.
I uploaded crash logs here.
It seems the weak NSMapTable<NSString, id>
cause the issue ? Strange...
I'm sure I place a lock between each read/write to that NSMapTable
, but this still cause issue ?
Did you try to trigger a set method (-[SDImageCache setConfig:]
) to the SDImageCache.config
during runtime at different thread ? This API is actually not correct in 4.x. It's a readonly property, but however we can not change any public header because we follow semversion. In 5.x we fixed it...
I don't set any configuration for SD
, I just use it as it is.
@Robot2037 What previous version of SDWebImage did you use ? I remember this weak memory cache feature is available since SDWebImage 4.3.1 via #2228. Did you use any version between 4.3.1-4.4.2 version ?
For temporary workaround, try disable weak cache using shouldUseWeakMemoryCache = NO
. I'll try to find out the reason and provide a better solution.
@dreampiggy Hard to say, I don't force any version in Cocoapods, so I always use the last available version. I just know date when I made release build and uploaded it to iTunes. Every time before making release build, I run pod update
command to update all libraries.
There are dates for three last app builds:
app version | build date |
---|---|
2.4.5 | 2018-10-01 |
2.4.4 | 2018-07-20 |
2.4.3 | 2018-05-18 |
Regarding Relases page, SD 4.4.2
was released 2018-07-18
, that means app version 2.4.4
and 2.4.5
uses SD 4.4.2
. This crash started to appear 5 days ago for both versions. Strange there were no crashes until then and also there is no crash in our other apps using the same SD
version. There is screenshot from Crashlytics.
@Robot2037 After some debug, I can confirm now, the crash happens when NSMapTable
empty the keys
, in SD
, keys
is NSString
of the image url, when we remove the objects, runtime calls objc_msgSend
, the selector is release
to release the keys
, in objc_msgSend
, when try to get isa
of the parameter self
, then crash, because key
already released.
The crash procedure just like above I describe. But the actual reason why it would be released in advance is unknown. In theory, we use strong
to store the key
, would not be released before NSMapTable
release it.
@dreampiggy , I have a thought to try to fix it, before we call - setObject:forKey:
of NSMapTable
, we do a mutableCopy
of key
if key
is NSString
, let NSMapTable
to manage the memory of key
. I don't use NSPointerFunctionsCopyIn
because it would not always do copy
. What's your opinion? 🤔
Any news on this one? It is very strange there are no more crashes since I reported it here. It suddenly appeared on 15th October and lasted until 18th October, caused 36 crashes in total, and then disappeared. No more reports for last week and also there were no reports before 15th October. At first I thought it was related to new app release, but later I realized even the previous app version was using same SD
version. This issue probably only happens during very specific application state or situation.
@Robot2037 Yeah, it's strange, because the keys is valid when remove objects in NSCache
, but invalid when empty keys in NSMapTable
. So seems it's a weird system bug? I don't know wether we need to fix it, but I have a thought in previous comment to try to fix it. Let's hear other guys opinion. cc @bpoplauschi .
From what I see, the keys have strong references, where the values are weak.
self.weakCache = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory valueOptions:NSPointerFunctionsWeakMemory capacity:0];
Any chance the UIImage
stored in the NSMapTable
is deallocated and then sent a message?
@bpoplauschi Yeah, at first, I suspect the issue happened in weak operation, but after @Robot2037 provide some crash log, I debug refer to the offset of crash line, the crash happened when empty keys. Detailed analyze I already comment here.
Really strange. It seems a Apple implementation bug ? Because we use strong-weak table, the key should follow the strong (retain + 1) semantic behavior, and should not been released before we manual remove that.
Your attempt (or hack ?) may work from the theory and we can have a try. We just do a real copy of the original string for key (I think using +[NSSring stringWithFormat:]
is better, mutable may not so safe, many of our API is avoid using NSMutableString
...).
@Robot2037 Is this appear for all firmware version ? (From iOS 8+ ~ iOS 12), or only some specify version or jailbroken device ? I want to check if this is the SDK bug or some designed behavior.
If the key is TaggedPointer
, the key will not follow the strong (retain + 1) semantic behavior, According to https://opensource.apple.com/source/objc4/objc4-723/runtime/NSObject.mm.auto.html
objc_retain(id obj)
{
if (!obj) return obj;
if (obj->isTaggedPointer()) return obj;
return obj->retain();
}
But if it's a tagged pointer, the objc_release
will also ignore it. And objc_msgSend
will also handle this situation. I'm still confused what this is going on wrong with that map table usage.
__attribute__((aligned(16)))
void
objc_release(id obj)
{
if (!obj) return;
if (obj->isTaggedPointer()) return;
return obj->release();
}
It's not a 100% reproduable issue, maybe there are some other issue cause this crash ?
Yeah, This should not be a problem for TaggedPointer, I try to debug this issue on my iOS8 device for firmware reasons.
@dreampiggy Issue was reported on devices running iOS 10 up to iOS 12, both iPhone and iPad. There were no jailbroken devices reported.
@dreampiggy @bpoplauschi I want to try to fix it like below, what's your guys opinion? 🤔
[self.weakCache setObject:obj forKey:[[key mutableCopy] copy]];
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, please make sure it is up to date and if so, add a comment that this is still an issue to keep it open. Thank you for your contributions.
@zhongwuzw we can try this idea :)
Should we reopen?
New Issue Checklist
Issue Info
Issue Description and Steps
I'm getting following error while application tries to clean memory cache after entering background state. This crash started to appear since SDWebImage 4.4.2, but doesn't happen every time.
Here is crash stacktrace: