Re: Workaround for framework leaks?

Christiaan Hofman <[email protected]> Sun, 15 Aug 2010 23:22:48 +0200
Newsgroups gmane.comp.macosx.devel
Message-ID <[email protected]>
On Aug 15, 2010, at 16:25, Gregory Weston wrote:

> (Sorry. Had originally, unintentionally, not done a reply-to-all. I infer that it's okay to re-add the list as a recipient because the last part of your message included an appeal that obviously was addressed to a collective.)
> 

Yea, I sometimes get that as well, I find it annoying this list doesn't automatically reply to the list.

>>>> or just run the following code that should crash in memory-managed code:
>>>> 
>>>> [[[[[PDFAnnotationCircle alloc] init] autorelease] border] release];
>>>> 
>>>> You can't say that those results are as they should be.
>>> 
>>> You can't say that they aren't. What you're consistently reproducing is that the default border object associated with a newly-created annotation object is being retained more than you expect for reasons of which you're unaware. You don't actually know, though, that it's unintentional so you don't know that it's a leak.
>> 
>> I am NOT looking at the retainCount or something. I am looking at Leaks, which tells me that the object is leaked (if you don't believe the result, than you're basically saying that Leaks is wrong and a useless program). Moreover, the second statement SHOULD crash in memory managed code (or when I use NSZombiesEnabled it should report a zombie eventually), and it doesn't (and I do wait long enough). This OBVIOUSLY cannot be intentional. Note that after the first statement the object that's leaked is not even referenced anywhere anymore. If you don't use autorelease but multiple statements it's even more obvious.
> 
> Actually, leaks has been known to report false positives under certain rare circumstances.

I see no real reason to think such a rare situation can be happening here, as this is such a straightforward situation. See also below..

> And I dispute that the line of code you present above *should* crash in that there are numerous trivial scenarios in which it won't.

First, note that I gave the code as an illustration that should be sufficient to show you where the bug occurs. It is NOT, I repeat NOT, the test itself. It's only the starting point, the actual tests itself are much more involved and details. Again, the code is only a hint to point to where the problem is. I am not going to detail all the leaks and gdb etc tests that I went through to get to my conclusion, my point is that you could easily go through that yourself. I assume you have an understanding as to how you determine whether there are leaks, and this hint should allow you to repeat the analysis.

If you are talking about autorelease, that's covered, which you can easily see for yourself (that's my point). For instance, it also occurs when I enclose in a local autorelease pool. 

> 
> It comes down to two things:
> The initial border object, by the time you have access to it, has been retained more than you expect.

Not me, leaks. And again, I am NOT basing this discussion on an observation of retainCount. 

> Leaks is unable to find anything that appears to be a reference to that object allowing that extra retain to be balanced.
> 

From any perspective that makes any sense, the initial border should not be referenced by anything after it's been replaced and the autorelease pool is drained. And leaks also confirms that fact.

> It *may* be a leak; I don't claim it definitely isn't. It may also, however, be the result of an atypical but legit design decision. That's part of why trying to second guess this and "fix" it in your code is so nasty. (The other, of course, being that you don't know when it will be fixed and how your fix will behave then.)

I cannot think of ANY sensible design decision whatsoever in this case. Note that the case is VERY clean and simple.

Moreover, memory usage does build up in an unreasonable way when using this code a lot. That's what leads to the bug reports from my users.

So I don't think one can reasonably say this isn't a leak. Seriously, if I cannot conclude after all I've seen that this isn't a genuine leak than you're saying that it's in principle not possible to determine a leak. Sorry,  I don't buy that. So again, from what I've seen, this is as clear a case for a leak as there can be, I gave to code so you can confirm if you don't believe me, THAT was the purpose of the code lines I gave. But if you don't want to check AND don't trust me, well...

> 
>>>>> Failing that, and again making absolutely sure that this is *really* a bug, I would say that you might be able to get by by coming up with a very rigorous and detectable set of circumstances under which the bug exists and deal with the issue only when those circumstances hold true.
>>>> 
>>>> Well, and here is the pain: where can I get such tests? Certainly retainCount is useless.
>>> 
>>> You determine them yourself.
>>> 
>>> It's interesting to me that you declare retainCount useless (and you're right there, of course) given that your "leak" manifests as a discrepancy in the expected value returned by retainCount.
>> 
>> See my answer above: no, I do NOT conclude this based on retainCount.
> 
> I didn't say you had. You made quite clear that you were relying on leaks output. Read more carefully.

Uhm, sorry, but Leaks does NOT 'just' look at the retainCount. It may use it, but in a much more sophisticated way, and moreover it takes into account the reference count, as you said. So also it takes into account autorelease, indirectly. 

> 
>>>>> Needless to say, one of those circumstances is going to be the precise versions of the specific framework(s) involved. If you detect any deviations from those conditions, don't mess with it.
>>>> 
>>>> I don't think Apple will tell me in which version of the framework they're gonna fix this?
>>> 
>>> Nor would I, even presuming that they agree that it's a bug. In a situation like this it's up to you to determine the range of systems on which you can confirm the problem exists and to address it as you feel necessary on those systems. You'll have to take for granted that in circumstances for which you haven't personally confirmed the manifestation of the bug that the bug isn't there.
>> 
>> So you're basically confirming there's really nothing I can do, because anything I can do will be useless the next time Apple comes with a bug fix release. Pity.
> 
> Yes. And nothing specific to PDFKit, or OS X, or Apple. This is the case any time code outside your control has a bug. There's no truly safe way for you to fix it. You can avoid it or you can be very careful and very narrow about applying your fix and cross your fingers.
> 
>> And how can I figure out the version of PDFKit?
> 
> Frameworks are executable bundles. They've got version numbers.
> 
> 	CFBundleRef b = CFBundleGetBundleWithIdentifier(CFSTR("com.apple.PDFKit"));
> 	UInt32 v = CFBundleGetVersionNumber(b);
> 
> or
> 
> 	NSBundle* b = [NSBundle bundleWithIdentifier:@"com.apple.PDFKit"];
> 	NSDictionary* d = [b infoDictionary];
> 	NSString* v = [d objectForKey:(NSString*)kCFBundleVersionKey];
> 

Sorry, I tried something like this, but somehow I got nil. Perhaps I mistyped something, because now it works.

Christiaan