1

I'm using QTouchposeApplication library (particularly a class TRTouchposeApplication from develop branch).

There were crashes in one of the execution paths in master branch, and the author fixed them in develop branch. However, one of the crashes is still present.


Let's say we have a CF-dictionary, which stores UITouch objects without retaining them:

// Dictionary of touches being displayed. Keys are UITouch pointers and values are UIView pointers that visually represent
// the touch on-screen. (A CFMutableDictionaryRef is used because NSDictionary requries its keys to conform to the
// NSCopying protocol and UITouch doesn't. We don't need to retain either the UITouch or UIView instances because UITouch
// objects are persistent throughout a multi-touch sequence, and the UIViews are retained by their superview.)
CFMutableDictionaryRef _touchDictionary;

We add UITouch to the dictionary like this:

CFDictionarySetValue(_touchDictionary, (__bridge const void *)(touch), (__bridge const void *)(someUIView));

Then at some point UITouch is released by the system (I'm using ARC in my project). So when we try to "unwrap" the touch, it crashes. I've got a message -[UITouch retain]: message sent to deallocated instance with Xcode Address Sanitizer:

enter image description here

So the reason of the crash is that we are storing ARC-objects as void*, so __bridge crashes if this object was deallocated.

I've seen __bridge, __bridge_transfer, __bridge_retained, but didn't see smth like bridge_TRY.

In order to write a patch for this crash, I'd like to know:

  1. Can we "safely bridge" somehow in this case, and get a result UITouch = nil, if touch was deallocated.
  2. Can we somehow subscribe for "UITouch" deallocation, so that we delete a corresponding UITouch from CFDictionary before the touch object will be deleted and pointer invalid?
  3. Are there the other ways to handle this?
olha
  • 2,132
  • 1
  • 18
  • 39
  • 1
    That library was created 9 years ago and was last updated 5 years ago. Your energy will be better spent recreating the single class that it has rather than trying to make it work. – EmilioPelaez May 25 '21 at 14:38
  • 2
    The more I look at I the worse it gets. I don't see why they would use `CFDictionary` instead of `NSDictionary`. – EmilioPelaez May 25 '21 at 14:41
  • @EmilioPelaez The comment covers it well. NSDictionary keys are required to conform to NSCopying (because keys are copied into the dictionary, not retained), and UITouch does not conform (and is not intended to be copied directly). – Rob Napier May 25 '21 at 16:33
  • @RobNapier I guess I missed that comment, but it was still a bad idea back then. `NSDictionary` copies the key to avoid a situation where the key is no longer available, which is what seems to be happening right now. I stand by mi first comment, it's a really outdated library and any energy would be better spent re-writing it than trying to figure out how to adapt it. – EmilioPelaez May 25 '21 at 17:24

3 Answers3

2

You need use NSMapTable instead of dictionary if you need store weak pointers at keys or values

Cy-4AH
  • 4,370
  • 2
  • 15
  • 22
1

In line 83 you use cycle for for check each dictionary value, but in line 91 you change dictionary size. So better to add other outputDictionary at the line 77, that would be equal _touchDictionary, and in line 91 remove data from outputDictionary, and in line 95 set _touchDictionary = outputDictionary back.

It would be more safety

  • Thanks! I'll try and accept if this eliminates the "__bridge" crash (I'm not sure this is the only reason of a crash) – olha May 25 '21 at 14:45
1

Looking at this code, I believe the dictionary should be retaining the touch and the view. The simple rule of memory management is "retain what you care about and release when you don't care any more." There's rarely a reason to rely on some other object to retain things for you.

The advantage of "non-retaining" CFDictionaries is that they allow you to store things that don't respond to retain and release (malloced memory for example). But generally, you should enable normal memory management on them by using the standard callbacks. Instead of this:

_touchDictionary = CFDictionaryCreateMutable(NULL, 10, NULL, NULL);

Use:

_touchDictionary = CFDictionaryCreateMutable(NULL, 10,
                                             &kCFTypeDictionaryKeyCallBacks, 
                                             &kCFTypeDictionaryValueCallBacks);

Then the dictionary will retain its keys and values for you.

Do be careful about holding onto a UITouch after the gesture is complete. The docs forbid this:

A touch object persists throughout a multi-touch sequence. You may store a reference to a touch while handling a multi-touch sequence, as long as you release that reference when the sequence ends. If you need to store information about a touch outside of a multi-touch sequence, copy that information from the touch.

But I don't think you're necessarily doing that incorrectly. The crash is at the point where you remove the touch, which I assume is at the end of the sequence.

Just to touch on some of your other ideas:

Can we "safely bridge" somehow in this case, and get a result UITouch = nil, if touch was deallocated.

Yes, but that requires weak pointers, which is probably too big a change for this. Cy-4AH's NSMapTable suggestion does this, but you'd need to make changes throughout the code.

Can we somehow subscribe for "UITouch" deallocation

Yes, but don't. :D You can attach an associated object that has its own dealloc, and it will fire when the object it's attached to is deallocated, and then you can do things in response. But that's too much for this problem IMO. If you want an example, see PMKVObserverDeallocSpy.

Rob Napier
  • 286,113
  • 34
  • 456
  • 610
  • Thank you for the `associated object`! Never heard of this. Regarding the retaining of the touch, I'll consider it. – olha May 25 '21 at 18:00