Fix a rare crash in TSKSPKIHashCache#348
Fix a rare crash in TSKSPKIHashCache#348fabianmuecke wants to merge 5 commits intodatatheorem:masterfrom
Conversation
|
I do see the same issue in my project. What holds this pr back from merging? |
|
Does anyone have a crash log they can share? :) Edit: oops, from #347: |
|
@NSExceptional -- here is another
|
|
@fabianmuecke Do you mind if I edit your PR? |
Not at all. Feel free to do whatever it takes to get a fix merged. 🙂 |
|
@NSExceptional @aztrmobile - Please let know when it's expected to get merged. Thanks. |
|
@ashok-meena-engcp Sorry for the delay, working on getting this merged ASAP behind the scenes |
|
|
||
| dispatch_sync(_lockQueue, ^{ | ||
| cachedSubjectPublicKeyInfo = self->_spkiCache[certificateData]; | ||
| }); | ||
| NSData *cachedSubjectPublicKeyInfo = self->_spkiCache[certificateData]; |
There was a problem hiding this comment.
_hashSubjectPublicKeyInfoFromCertificate: is entirely in the lock queue now
| dispatch_barrier_sync(_lockQueue, ^{ | ||
| self->_spkiCache[certificateData] = subjectPublicKeyInfoHash; | ||
| }); | ||
| self->_spkiCache[certificateData] = subjectPublicKeyInfoHash; |
There was a problem hiding this comment.
_hashSubjectPublicKeyInfoFromCertificate: is entirely in the lock queue now
| } | ||
| else { | ||
| dispatch_async(dispatch_get_main_queue(), updateCacheBlock); | ||
| if (self.spkiCacheFilename.length && isProtectedDataAvailable()) { |
There was a problem hiding this comment.
This will block to call out to the main queue, but that is our last resort, short of refactoring this method and several others to be totally asynchronous, which would be a massive undertaking. The main queue is a well oiled machine and shouldn't be busy for more than a few ms at any given time, and if it is, the app has much bigger problems than blocking this queue waiting on it
6 participants
This PR fixes #347 by adding a missing
dispatch_syncwhen reading the cache on the main dispatch queue to write it to the file system. This should be safe, as the onlydispatch_barrier_synccall does not access the main queue, so cannot cause a deadlock.I tried to find a test that would replicate the issue and found one, that will randomly (maybe once out of 1000 runs) achieve that. All attempts to improve the reliability of the test were futile. I added it the the PR anyway. Feel free to remove, if you think it's useless.