subject:"Analyser reports memory leak… where\?"

Here's some code for which the Analyser reports potential leak of an object 
stored into 'eventTypes'. I don't see it.

I didn't write this code, so I'm reluctant to change it even though I would 
have written it a bit differently. mEventTypes is an ivar.

- (void)setEventTypes:(NSDictionary*)eventTypes
{
if (eventTypes != mEventTypes)
{
[mEventTypes release];
mEventTypes = [eventTypes retain];
}
InitializePrefsForEventTypeNames();
}

- (NSDictionary*)eventTypes
{
if (mEventTypes == nil)
{
[self loadNib];

NSDictionary* eventTypes = [self newEventTypes];
[self setEventTypes:eventTypes];
[eventTypes release];
}

return mEventTypes; //- analyser complains here
}



- (NSDictionary*)newEventTypes
{
//[code deleted that presets contents of 'eventTypes']

// Method name begins with new; clients are responsible for releasing.
return [[NSDictionary alloc] initWithDictionary:eventTypes];
}



--Graham
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

I don't see it either, if you click on the error doesn't it give you a diagram 
with arrows telling you how it arrives at that conclusion? 

On 12 Sep, 2013, at 5:35 pm, Graham Cox graham@bigpond.com wrote:

 Here's some code for which the Analyser reports potential leak of an object 
 stored into 'eventTypes'. I don't see it.
 
 I didn't write this code, so I'm reluctant to change it even though I would 
 have written it a bit differently. mEventTypes is an ivar.
 
 - (void)setEventTypes:(NSDictionary*)eventTypes
 {
   if (eventTypes != mEventTypes)
   {
   [mEventTypes release];
   mEventTypes = [eventTypes retain];
   }
   InitializePrefsForEventTypeNames();
 }
 
 - (NSDictionary*)eventTypes
 {
   if (mEventTypes == nil)
   {
   [self loadNib];
   
   NSDictionary* eventTypes = [self newEventTypes];
   [self setEventTypes:eventTypes];
   [eventTypes release];
   }
   
   return mEventTypes; //- analyser complains here
 }
 
 
 
 - (NSDictionary*)newEventTypes
 {
   //[code deleted that presets contents of 'eventTypes']
 
   // Method name begins with new; clients are responsible for releasing.
   return [[NSDictionary alloc] initWithDictionary:eventTypes];
 }
 
 
 
 --Graham
 ___
 
 Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)
 
 Please do not post admin requests or moderator comments to the list.
 Contact the moderators at cocoa-dev-admins(at)lists.apple.com
 
 Help/Unsubscribe/Update your Subscription:
 https://lists.apple.com/mailman/options/cocoa-dev/rols%40rols.org
 
 This email sent to r...@rols.org

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 11:43 AM, Roland King r...@rols.org wrote:

 I don't see it either, if you click on the error doesn't it give you a 
 diagram with arrows telling you how it arrives at that conclusion? 


Yes, but that doesn't make sense either. Here's a screen shot from Xcode: 
http://apptree.net/images/leak.png

It seems to have simply ignored the call to -release. A bug in the analyser? 
Xcode 4.6.2

-Graham


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12 Sep, 2013, at 5:50 pm, Graham Cox graham@bigpond.com wrote:

 
 On 12/09/2013, at 11:43 AM, Roland King r...@rols.org wrote:
 
 I don't see it either, if you click on the error doesn't it give you a 
 diagram with arrows telling you how it arrives at that conclusion? 
 
 
 Yes, but that doesn't make sense either. Here's a screen shot from Xcode: 
 http://apptree.net/images/leak.png
 
 It seems to have simply ignored the call to -release. A bug in the analyser? 
 Xcode 4.6.2


I can't see anything wrong and you're right the graph makes no sense so I'd 
have to say bug in the analyser. I might try various voodoo like clean building 
etc incase it's got itself in a mess but that just looks wrong. 
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Simone Tellini


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Simone Tellini

I hate it when Thunderbird manages to throw away the content of the mails I 
send to the list! Let's try again...

Il giorno 12/set/2013, alle ore 11:35, Graham Cox graham@bigpond.com ha 
scritto:

 Here's some code for which the Analyser reports potential leak of an object 
 stored into 'eventTypes'. I don't see it.
 
 I didn't write this code, so I'm reluctant to change it even though I would 
 have written it a bit differently. mEventTypes is an ivar.
 
 - (void)setEventTypes:(NSDictionary*)eventTypes
 {
if (eventTypes != mEventTypes)
{
[mEventTypes release];
mEventTypes = [eventTypes retain];
}
InitializePrefsForEventTypeNames();
 }
 
 - (NSDictionary*)eventTypes
 {
if (mEventTypes == nil)
{
[self loadNib];

NSDictionary* eventTypes = [self newEventTypes];
[self setEventTypes:eventTypes];
[eventTypes release];
}

return mEventTypes;//- analyser complains here
 }
 

is there a [mEventTypes release] in -dealloc?

Otherwise, this might be the potential problem it complains about: 
-eventTypes can create a new object which never goes away (since its name 
doesn't imply a +1 return, the caller is not supposed to release it).

-- 
Simone Tellini
http://tellini.info/
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 12:20 PM, Simone Tellini cocoa-...@tellini.info wrote:

 s there a [mEventTypes release] in -dealloc?


Yes there is.

--Graham
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 12:34 PM, Bill Cheeseman wjcheese...@gmail.com wrote:

 The retain count is still +1 at the point where the error is reported.


But shouldn't it take into account that it was assigned to an ivar that is 
retained until -dealloc? Or is it trying to tell me that it will leak if the 
enclosing object happens to leak? If so, that would mean thousands of false 
positives in most code, which doesn't appear to happen. This particular object 
is a singleton that is never deallocated explicitly in fact - is that it?

--Graham


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

That's not even what the error message is saying, it's trying to tell you, as 
you summarized earlier, that the temporary variable you are assigning to and 
properly releasing is being called out as a leak, it's not complaining about 
the ivar.

 
On 12 Sep, 2013, at 6:52 pm, Graham Cox graham@bigpond.com wrote:

 
 On 12/09/2013, at 12:34 PM, Bill Cheeseman wjcheese...@gmail.com wrote:
 
 The retain count is still +1 at the point where the error is reported.
 
 
 But shouldn't it take into account that it was assigned to an ivar that is 
 retained until -dealloc? Or is it trying to tell me that it will leak if the 
 enclosing object happens to leak? If so, that would mean thousands of false 
 positives in most code, which doesn't appear to happen. This particular 
 object is a singleton that is never deallocated explicitly in fact - is that 
 it?
 
 --Graham
 
 
 ___
 
 Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)
 
 Please do not post admin requests or moderator comments to the list.
 Contact the moderators at cocoa-dev-admins(at)lists.apple.com
 
 Help/Unsubscribe/Update your Subscription:
 https://lists.apple.com/mailman/options/cocoa-dev/rols%40rols.org
 
 This email sent to r...@rols.org

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 12:56 PM, Roland King r...@rols.org wrote:

 That's not even what the error message is saying, it's trying to tell you, as 
 you summarized earlier, that the temporary variable you are assigning to and 
 properly releasing is being called out as a leak, it's not complaining about 
 the ivar.


So is it a leak or not? More confused than ever….

--Graham
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12 Sep, 2013, at 7:07 pm, Graham Cox graham@bigpond.com wrote:

 
 On 12/09/2013, at 12:56 PM, Roland King r...@rols.org wrote:
 
 That's not even what the error message is saying, it's trying to tell you, 
 as you summarized earlier, that the temporary variable you are assigning to 
 and properly releasing is being called out as a leak, it's not complaining 
 about the ivar.
 
 
 So is it a leak or not? More confused than ever….
 
 --Graham

No it's not a leak - I still think the analyser is lying. 

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

In my understanding, Analyze is meant to be very literal, even simple-minded. 
Your method returns a retained object but you did not put new or copy in 
the method name. It assumes that you were following the naming convention and 
that your omission of new or copy was intentional. It therefore reported to 
you that the return of a retained object is an error. It is telling you that 
your code is wrong only in this very literal and simple-minded sense.

In fact, your comment in the last method, which does have new in its name, 
demonstrates that you are aware of the naming convention. Why did you not 
follow the convention in the other method? Add new or copy to the name and 
build it again. The Analyze error went away, right?

I figured this out a couple of years ago when I first applied Analyze to my own 
code. I spent hours parsing my memory management code and concluded that it was 
correct despite the Analyze errors. Then, on a whim, I added new and copy 
to several method names, and suddenly Analyze agreed that my code was correct. 
It is not smart enough to look ahead in the logic to see that -dealloc took 
care of the second retain. I'm not enough of a computer scientist to know if 
that is even possible without impractical levels of analysis.

At the time, the fact that Cocoa had started following a naming convention that 
had until then applied only to Core Foundation was documented exactly nowhere. 
I haven't looked since then, so I don't know whether it is now documented 
correctly.

Bill Cheeseman

On Sep 12, 2013, at 6:52 AM, Graham Cox graham@bigpond.com wrote:

 But shouldn't it take into account that it was assigned to an ivar that is 
 retained until -dealloc? Or is it trying to tell me that it will leak if the 
 enclosing object happens to leak? If so, that would mean thousands of false 
 positives in most code, which doesn't appear to happen. This particular 
 object is a singleton that is never deallocated explicitly in fact - is that 
 it?

-- 

Bill Cheeseman - b...@cheeseman.name

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Fwd: Analyser reports memory leak… where?

I neglected to send this to the list. It's important enough that I think I 
should.

Bill Cheeseman

Begin forwarded message:

 From: Bill Cheeseman wjcheese...@gmail.com
 Subject: Re: Analyser reports memory leak… where?
 Date: September 12, 2013 6:34:05 AM EDT
 To: Graham Cox graham@bigpond.com

 The -eventTypes method sends the -setEventTypes: message, which retains the 
 object and assigns it to the mEventTypes iVar. Then -eventTypes returns the 
 mEventTypes iVar without autoreleasing it, and -eventTypes doesn't have 
 new, copy, or whatever in its name. The release of the eventTypes local 
 variable in -eventTypes only offsets the first retain implicit in the -alloc 
 message in -newEventTypes. The retain count is still +1 at the point where 
 the error is reported.

 Bill Cheeseman

 On Sep 12, 2013, at 5:35 AM, Graham Cox graham@bigpond.com wrote:

 Here's some code for which the Analyser reports potential leak of an object 
 stored into 'eventTypes'. I don't see it.

 I didn't write this code, so I'm reluctant to change it even though I would 
 have written it a bit differently. mEventTypes is an ivar.

 - (void)setEventTypes:(NSDictionary*)eventTypes
 {
  if (eventTypes != mEventTypes)
  {
  [mEventTypes release];
  mEventTypes = [eventTypes retain];
  }
  InitializePrefsForEventTypeNames();
 }

 - (NSDictionary*)eventTypes
 {
  if (mEventTypes == nil)
  {
  [self loadNib];

  NSDictionary* eventTypes = [self newEventTypes];
  [self setEventTypes:eventTypes];
  [eventTypes release];
  }

  return mEventTypes; //- analyser complains here
 }

 - (NSDictionary*)newEventTypes
 {
  //[code deleted that presets contents of 'eventTypes']

  // Method name begins with new; clients are responsible for releasing.
  return [[NSDictionary alloc] initWithDictionary:eventTypes];
 }

 --Graham
 ___

 Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

 Please do not post admin requests or moderator comments to the list.
 Contact the moderators at cocoa-dev-admins(at)lists.apple.com

 Help/Unsubscribe/Update your Subscription:
 https://lists.apple.com/mailman/options/cocoa-dev/wjcheeseman%40gmail.com

 This email sent to wjcheese...@gmail.com

 -- 

 Bill Cheeseman - b...@cheeseman.name

-- 

Bill Cheeseman - b...@cheeseman.name

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 1:59 PM, Bill Cheeseman wjcheese...@gmail.com wrote:

 In my understanding, Analyze is meant to be very literal, even simple-minded. 
 Your method returns a retained object but you did not put new or copy in 
 the method name. It assumes that you were following the naming convention and 
 that your omission of new or copy was intentional. It therefore reported 
 to you that the return of a retained object is an error. It is telling you 
 that your code is wrong only in this very literal and simple-minded sense.


I do see this, but what I'm not quite getting is why this doesn't get reported 
for all sorts of other property getters that also correctly return retained 
objects (ivars, typically) that are not owned by the caller.

 In fact, your comment in the last method, which does have new in its name, 
 demonstrates that you are aware of the naming convention. Why did you not 
 follow the convention in the other method? Add new or copy to the name 
 and build it again. The Analyze error went away, right?


I didn't write this code, I wouldn't have done it this way - I generally avoid 
writing 'newXXX' methods to return objects that the caller has to release if I 
can help it. For some reason the author of this code did it this way.

In this case, -eventTypes is just a normal property getter which initializes 
the ivar lazily, using an internal 'new' method to do it. What's not clear to 
me is why it cares that the temporary variable, which is dealt with correctly, 
has a retain count of 1, having assigned to the property's ivar. How is that 
different to returning a retained property ivar at any other time? Changing the 
getter's name to include 'new' or 'copy' might shut up the analyser, but it's 
incorrect since a) the caller should NOT release the return value (that would 
leave the ivar dangling) and b) it breaks the convention for property getter 
names.


-

I've now slightly modified the code to do things a little more conventionally, 
refactoring the name of 'newEventTypes' to 'makeEventTypes' and returning an 
autoreleased object, which my -eventTypes getter no longer releases. The 
analyser is now happy, even though it is *still* returning the ivar with a +1 
retain count and nothing else has changed.

I guess it just isn't expecting code that follows a pattern that, while 
apparently legal, is not really commonly used.


--Graham



___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

Creative but no I don't think so. 

For a start that's not what the error says, if it cared less what the property 
setter setEventTypes: did then it would point out that increased the refcount 
by 1, the release decremented it by 1 and thus there was a net +1 in there. It 
doesn't do that, it doesn't mention that method at all. 

setEventTypes: takes a parameter and returns nothing. What it does with that 
parameter is entirely up to it, it may retain it if it wishes to be released 
later, or do any other thing it wants. If the analyser doesn't much like what 
setEventTypes: is doing, it would complain about setEventTypes:, which it's not 
doing, it's complaining about a caller. That method doesn't return anything and 
has no special naming so the analyser will presume it correctly to do nothing 
net-net to its argument. 

-eventTypes doesn't need the new or copy because it does not return an object 
owned by the caller. Whether the object it returns is already owned by itself 
to be released later is irrelevant, the refcount has not been increased before 
return to the caller so the method is correctly named and the analyser can 
properly assume it to be a +0 method. And apart from that it's not even called 
in the piece of code in question so that's not really relevant. 

If what you wrote were true, and the analyser wandered into void returning 
methods like property setters which by necessity retain their arguments  then 
it would warn everyone constantly about every property setter and it doesn't. I 
don't even believe as of 4.6 the analyser does go into methods, I believe this 
was covered in WWDC 2013. 

That code still doesn't have a leak in it and I don't know what about it 
triggers an analyser bug, but the analyser is just wrong here and it's wrong in 
a very simple common case. What's possibly even stranger is that the error 
message about eventTypes not being referenced in that code path is outside the 
scope eventTypes is even defined, so the thing really is confused. Which leads 
to one question, there isn't another in-scope variable called eventTypes is 
there? Static, another iVar, from a superclass, synthethized in some way .. 
something? If there were then that might explain the confusion (which 
eventTypes does [ eventTypes release ] refer to in that case). 




On 12 Sep, 2013, at 8:00 pm, Bill Cheeseman wjcheese...@gmail.com wrote:

 I neglected to send this to the list. It's important enough that I think I 
 should.
 
 Bill Cheeseman
 
 Begin forwarded message:
 
 From: Bill Cheeseman wjcheese...@gmail.com
 Subject: Re: Analyser reports memory leak… where?
 Date: September 12, 2013 6:34:05 AM EDT
 To: Graham Cox graham@bigpond.com
 
 The -eventTypes method sends the -setEventTypes: message, which retains the 
 object and assigns it to the mEventTypes iVar. Then -eventTypes returns the 
 mEventTypes iVar without autoreleasing it, and -eventTypes doesn't have 
 new, copy, or whatever in its name. The release of the eventTypes local 
 variable in -eventTypes only offsets the first retain implicit in the -alloc 
 message in -newEventTypes. The retain count is still +1 at the point where 
 the error is reported.
 
 Bill Cheeseman
 
 On Sep 12, 2013, at 5:35 AM, Graham Cox graham@bigpond.com wrote:
 
 Here's some code for which the Analyser reports potential leak of an 
 object stored into 'eventTypes'. I don't see it.
 
 I didn't write this code, so I'm reluctant to change it even though I would 
 have written it a bit differently. mEventTypes is an ivar.
 
 - (void)setEventTypes:(NSDictionary*)eventTypes
 {
 if (eventTypes != mEventTypes)
 {
 [mEventTypes release];
 mEventTypes = [eventTypes retain];
 }
 InitializePrefsForEventTypeNames();
 }
 
 - (NSDictionary*)eventTypes
 {
 if (mEventTypes == nil)
 {
 [self loadNib];
 
 NSDictionary* eventTypes = [self newEventTypes];
 [self setEventTypes:eventTypes];
 [eventTypes release];
 }
 
 return mEventTypes; //- analyser complains here
 }
 
 
 
 - (NSDictionary*)newEventTypes
 {
 //[code deleted that presets contents of 'eventTypes']
 
 // Method name begins with new; clients are responsible for releasing.
 return [[NSDictionary alloc] initWithDictionary:eventTypes];
 }
 
 
 
 --Graham
 ___
 
 Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)
 
 Please do not post admin requests or moderator comments to the list.
 Contact the moderators at cocoa-dev-admins(at)lists.apple.com
 
 Help/Unsubscribe/Update your Subscription:
 https://lists.apple.com/mailman/options/cocoa-dev/wjcheeseman%40gmail.com
 
 This email sent to wjcheese...@gmail.com
 
 -- 
 
 Bill Cheeseman - b...@cheeseman.name
 
 
 -- 
 
 Bill Cheeseman - b...@cheeseman.name
 
 ___
 
 Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)
 
 Please do

Re: Analyser reports memory leak… where?


On 12/09/2013, at 3:16 PM, Roland King r...@rols.org wrote:

 Which leads to one question, there isn't another in-scope variable called 
 eventTypes is there? Static, another iVar, from a superclass, synthethized in 
 some way .. something? If there were then that might explain the confusion 
 (which eventTypes does [ eventTypes release ] refer to in that case). 


That did occur to me as well, and I went and looked - nothing found (apart from 
the method name itself). Just to be sure, I changed the temporary variable name 
(to 'et') and the analyser now complained about et.

*shrugs*

--Graham


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Fritz Anderson

On 12 Sep 2013, at 4:35 AM, Graham Cox graham@bigpond.com wrote:

 Here's some code for which the Analyser reports potential leak of an object 
 stored into 'eventTypes'. I don't see it.
 
 I didn't write this code, so I'm reluctant to change it even though I would 
 have written it a bit differently. mEventTypes is an ivar.
 
 - (void)setEventTypes:(NSDictionary*)eventTypes
 {
   if (eventTypes != mEventTypes)
   {
   [mEventTypes release];
   mEventTypes = [eventTypes retain];
   }
   InitializePrefsForEventTypeNames();
 }
 
 - (NSDictionary*)eventTypes
 {
   if (mEventTypes == nil)
   {
   [self loadNib];
   
   NSDictionary* eventTypes = [self newEventTypes];
   [self setEventTypes:eventTypes];
   [eventTypes release];
   }
   return mEventTypes; //- analyser complains here
 }
 
 - (NSDictionary*)newEventTypes
 {
   //[code deleted that presets contents of 'eventTypes']
 
   // Method name begins with new; clients are responsible for releasing.
   return [[NSDictionary alloc] initWithDictionary:eventTypes];
 }

I tried a Foundation-tool project that included a class with the methods you 
show, dummying the methods and functions not shown as empty 
void-returning-void, and dictionary data as one string-keying-string. ARC off.

Running this through a later version of clang showed no analyzer errors. I 
think it's a bug in the version of clang you're using. Or: clang does 
cross-function checks to one degree or another. Can you make a minimal example, 
and add the code you omitted here to see what makes the analyzer unhappy?

I assume you expanded the note and examined the code path on which the compiler 
saw a potential problem?

— F


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Alex Zavatone

Is there any reference on the methods used for how the analyzer does its leak 
detection/identification?  If we had access to that, it should be really easy 
to pinpoint why the code is causing that error.

Cheers.

On Sep 12, 2013, at 9:57 AM, Graham Cox wrote:

 
 On 12/09/2013, at 3:16 PM, Roland King r...@rols.org wrote:
 
 Which leads to one question, there isn't another in-scope variable called 
 eventTypes is there? Static, another iVar, from a superclass, synthethized 
 in some way .. something? If there were then that might explain the 
 confusion (which eventTypes does [ eventTypes release ] refer to in that 
 case). 
 
 
 That did occur to me as well, and I went and looked - nothing found (apart 
 from the method name itself). Just to be sure, I changed the temporary 
 variable name (to 'et') and the analyser now complained about et.
 
 *shrugs*
 
 --Graham
 
 
 ___
 
 Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)
 
 Please do not post admin requests or moderator comments to the list.
 Contact the moderators at cocoa-dev-admins(at)lists.apple.com
 
 Help/Unsubscribe/Update your Subscription:
 https://lists.apple.com/mailman/options/cocoa-dev/zav%40mac.com
 
 This email sent to z...@mac.com


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On Sep 12, 2013, at 11:11 AM, Graham Cox graham@bigpond.com wrote:

 On 12/09/2013, at 5:00 PM, Bill Cheeseman wjcheese...@gmail.com wrote:
 
 Fix the name and the Analyzer is happy. No need to change the code, because 
 the code is correct
 
 Except that 'fixing' the name would make the code incorrect, because then it 
 isn't a valid getter name.
 
 If you did go ahead and name it thus, the caller would assume that it's their 
 job to release the returned value. If they did that, it would leave 
 mEventTypes dangling, and a later call to -release in -dealloc would crash 
 due to an overrelease. So renaming the method is wrong on both counts.


But -eventTypes is not in fact a valid getter because it returns the iVar 
retained but not autoreleased. The setter/getter convention requires getters to 
return autoreleased objects. So leaving it named according to the getter naming 
convention is wrong. Adding new would not only be correct but would also warn 
against using it as a getter. 

As written (no matter what it is named), it is in fact the caller's job to 
release it. That is presumably why you release it in -dealloc. If your class's 
-init method initially created the first object assigned to the iVar, whether 
by calling -eventTypes or by directly calling -newEventTypes and setting the 
iVar, then you are the owner and you must release it in -dealloc if not 
earlier. And if you call -eventTypes or -newEventTypes while substituting a new 
object for the old iVar object in midstream, you must release the old iVar 
object in midstream, too.

At it's core, that's what is really odd about the code. It uses a getter that 
does not comply with the getter convention.

-- 

Bill Cheeseman - b...@cheeseman.name

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Aaron Montgomery


On Sep 12, 2013, at 9:17 AM, Graham Cox graham@bigpond.com wrote:

 
 On 12/09/2013, at 6:07 PM, Kyle Sluder k...@ksluder.com wrote:
 
 Personally, I would avoid doing this, as it could cause reentrancy among
 KVO observers of eventTypes. Instead, I'd assign to mEventTypes directly
 from your lazy initializer. Or perhaps I would factor out the common
 setter code to a private helper method.
 
 Indeed, I have changed the code to do just this, which is what I would have 
 written in the first place (honest!). I'm not sure why it wasn't and changing 
 it appears to have not caused any problems.
 
 Hopefully this has the side effect of shutting the analyzer up.
 
 
 It does.
 
 I guess the thread is now really about why the analyser was thrown off by 
 this. Academic? Perhaps, but here's the bigger picture: I want to gradually 
 move my codebase to ARC, so in preparation for that, I have been going 
 through and trying to ensure the analyser is happy, since my understanding is 
 that ARC relies on the same analysis to do its thing correctly, so if the 
 analyser is getting it wrong, converting to ARC might introduce a bug here.


No, I don't think this is academic. The analyzer appears to have a real problem 
with using a setter inside a getter (both as exhibited with your original 
example and my more minimal example). If it is going to be responsible for 
adding all the memory management code to our code, we need to know that it can 
handle this situation or that it will refuse to convert to ARC when this 
situation is present. I'll try to see how it handles converting my minimal 
example to ARC.

Aaron  
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Lee Ann Rucker


On Sep 12, 2013, at 9:57 AM, Bill Cheeseman wrote:

 As I recall, it stated that the return [[x retain] autorelease] pattern is 
 preferred for getters and gave many reasons for preferring it. 

Coincidentally enough, we've just hit a case where that's bad - it's a 
complicated situation with ObjC++ and C++ object lifetimes, resulting in an 
accessor that uses that pattern being called as a result of that object being 
in dealloc. By the time the autorelease pool pops the object it's trying to 
release is long gone. It's KVO unregistering, so there's not much we can do 
about it other than be smarter in our accessors.
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Lee Ann Rucker


On Sep 12, 2013, at 1:49 PM, Greg Parker wrote:

 On Sep 12, 2013, at 1:45 PM, Lee Ann Rucker lruc...@vmware.com wrote:
 On Sep 12, 2013, at 9:57 AM, Bill Cheeseman wrote:
 As I recall, it stated that the return [[x retain] autorelease] pattern 
 is preferred for getters and gave many reasons for preferring it. 
 
 Coincidentally enough, we've just hit a case where that's bad - it's a 
 complicated situation with ObjC++ and C++ object lifetimes, resulting in an 
 accessor that uses that pattern being called as a result of that object 
 being in dealloc. By the time the autorelease pool pops the object it's 
 trying to release is long gone. It's KVO unregistering, so there's not much 
 we can do about it other than be smarter in our accessors.
 
 In some cases you can add an @autoreleasepool inside the object's -dealloc 
 method to catch these.

Ooh, yes, that works great! 

So many new language features, so little time...
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Tom Davie


On 12 Sep 2013, at 18:15, Daniel Höpfl ap...@hoepfl.de wrote:

 On 2013-09-12 17:52, Graham Cox wrote:
 I believe it does. I think your reading of the getter convention may
 be incorrect. If you can point to explicit documentation that states
 that the returned object must belong to an autorelease pool, I'll
 stand corrected, but that would be the first time I've ever heard that
 in 13 years of Cocoa programming!
 
 Even if there is such a rule:
 
 - (id) eventTypes
 {
// [self lazyInitEventTypes];
 
return [[mEventTypes retain] autorelease];
 }

It’s not a hard and fast rule, and in fact collection access does not do this, 
so it’s entirely possible to do things like:

id a = x[5];
[x removeObjectAtIndex:5];
[a crashMyProgram];

Tom Davie
___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On Sep 12, 2013, at 1:41 PM, Graham Cox graham@bigpond.com wrote:

 On 12/09/2013, at 6:57 PM, Bill Cheeseman wjcheese...@gmail.com wrote:
 
 
 
 The point remains, however, that the getter that started this thread 
 returned an object retained, and the caller therefore owns it and is 
 responsible for releasing it. 
 
 There's an inherent contradiction in this statement. If it's a getter, then 
 the caller has no responsibility for ownership of the returned object. If the 
 caller does have that responsibility, then it's not a getter. In fact, 
 -eventTypes in this case *is* a getter, or is intended to be.


The Advanced Memory Management Programming Guide I cited earlier says this 
about the Cocoa ownership rules, which I can't reconcile with your assertion 
that getters always leave the caller with no responsibility for ownership of 
the returned object.

You can take ownership of an object using retain
A received object is normally guaranteed to remain valid within the method it 
was received in, and that method may also safely return the object to its 
invoker. You use retain in two situations: (1) In the implementation of an 
accessor method or an init method, to take ownership of an object you want to 
store as a property value; and
I wonder if you are perhaps thinking of the Core Foundation Create Rule, which 
does say that a function named with get does not return a retained object (is 
returned by reference). If you can refer to any Cocoa documentation to the 
effect that getters never confer ownership in Cocoa, please pass it along.
-- 

Bill Cheeseman - b...@cheeseman.name

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Marcel Weiher

Hi Bill,

On Sep 12, 2013, at 18:57 , Bill Cheeseman wjcheese...@gmail.com wrote:

 As I recall, [Ali’s technote] stated that the return [[x retain] 
 autorelease] pattern is preferred for getters and gave many reasons for 
 preferring it. I understand that @synchronize generates getters that comply 
 with this preference, and I also recall that some of the early documentation 
 about properties discussed this convention.

I think you meant the ‘atomic' property modifier.   Note that the iOS team did 
not agree with Ali on this, due to the ginormous performance penalties in both 
CPU and memory: they made ‘nonatomic’ the standard for iOS, despite the fact 
that the language defaults to ‘atomic' and you therefore have to type 
‘nonatomic' every time.

Now considering the relative installed bases...

Marcel
p.s.: If I understand it correctly, ARC’s magic un-autoreleaser largely 
ameliorates the perf. problems

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 6:57 PM, Bill Cheeseman wjcheese...@gmail.com wrote:

I appreciate your taking an interest Bill, it's always interesting to get a 
range of views. But:

 The point remains, however, that the getter that started this thread returned 
 an object retained, and the caller therefore owns it and is responsible for 
 releasing it. 

There's an inherent contradiction in this statement. If it's a getter, then the 
caller has no responsibility for ownership of the returned object. If the 
caller does have that responsibility, then it's not a getter. In fact, 
-eventTypes in this case *is* a getter, or is intended to be.

  If the behavior of Analyze in this connection does still in fact change 
 depending on whether you include new or similar terms in the method name, 
 consistently with at least part of the Cocoa rule, one proper way to make the 
 ownership of the object returned by -eventTypes clear would be to put new 
 in its name.

Which would make it not a getter.


I think other contributors have actually got closer to the heart of the matter, 
which is that the getter calls the associated setter. That's probably a bad 
idea in general (certainly considering KVO, as Kyle pointed out, is very likely 
to cause an infinite loop), and I'm thinking that it should be a rule, that one 
does not do this. It could be that it's already an unwritten rule, baked into 
the assumptions made when the analyser itself was coded, and that's what's 
throwing it off. (Commenting out the setter line does make the analyser error 
go away). Normal getters, however they are coded with respect to the returned 
object, synthesized or whatever, do not call the associated getter, but as I 
said, this wasn't code I wrote. In fact, this particular module has been a 
constant source of pain since it was forced on me, but its days are numbered 
anyway.


--Graham



___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Steve Mills

On Sep 12, 2013, at 10:19:22, Aaron Montgomery eey...@monsterworks.com wrote:

 - (void)setObject:(NSObject *)object
 {
   [_object release];
   _object = [object retain];//3
 }

It might be that setObject: is not safe if object is the same as _object. The 
correct way to do this is:

[object retain];
[_object release];
_object = object;

--
Steve Mills
office: 952-818-3871
home: 952-401-6255
cell: 612-803-6157




___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Gary L. Wade

In your dealloc, you should release ivars before calling super dealloc. Ideally 
a crash will tell you that quickly; otherwise things could be very bad.
--
Gary L. Wade (Sent from my iPhone)
http://www.garywade.com/


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 5:27 PM, Steve Mills smi...@makemusic.com wrote:

 It might be that setObject: is not safe if object is the same as _object. The 
 correct way to do this is:
 
 [object retain];
 [_object release];
 _object = object;


Yep, though in the original code a test for equality brackets the assignment 
(again something I don't usually do unless the setter has side effects).

--Graham


___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Daniel Höpfl


On 2013-09-12 17:00, Bill Cheeseman wrote:

-eventTypes: is, in fact, a classic new method, though a bit oddly
written. All in the one method (by calls to utility methods), it
creates a new object with a refcount of 1 by calling +alloc
indirectly, increases its refcount by 1 more by calling
-setEventTypes:, decreases its refcount by 1 to balance the second
increase, and returns it with a refcount of 1 without autoreleasing
it. It does own the new object, and the method should have new in
its name.


I agree with Graham that the method must not be named new...
(because the caller is not to release the returned value).

I think there might be two reasons for the warning:

a) (I guess that not the reason): The analyser gets confused by
   seeing -setEventTypes:.

@Graham: What happens when you comment out the call to setEventTypes:?

  NSDictionary* eventTypes = [self newEventTypes];
  // [self setEventTypes:eventTypes];
  [eventTypes release];

Do you still get the warning?


b) The if (mEventTypes == nil) {} return mEventTypes; confuses the 
analyser:


   if (mEventTypes == nil)   // OK, mEventTypes might be nil here.
   {
  [self loadNib];// Might do anything, but due to the 
name: +/-0


  NSDictionary* eventTypes = [self newEventTypes];  // +1 for 
eventTypes

  [self setEventTypes:eventTypes];// Might do anything,
  // but due to the name: +/-0 for 
everything.

  [eventTypes release];   // -1 for eventTypes, fine.
   }

   return mEventTypes;
   // Wait. Why is the coder returning mEventTypes instead of nil?
   // Nobody created an object, but the coder assumes that there is an 
object
   // in mEventTypes now. Where did it come from? It must not be 
autoreleased
   // because this would result in a dangling pointer, so it MUST be +1 
(or more).

   // Might be a leak. Lets warn the coder.

Reasonable?

@Graham: Could you try the following:

- (BOOL)isEventTypesAvailable
{
   return mEventTypes != nil;
}

...
   if (![self isEventTypesAvailable])
   {
...

Bye,
   Daniel

@Bill: Sorry for sending this mail to you directly instead of to the 
list before. %-)

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On 12/09/2013, at 5:40 PM, Bill Cheeseman wjcheese...@gmail.com wrote:

 But -eventTypes is not in fact a valid getter because it returns the iVar 
 retained but not autoreleased.


How is it not valid?

A simple version of it would be:

- (id) eventTypes
{
 return mEventTypes;
}

which is perfectly valid. The rest of the code is just a lazy initialization 
for mEventTypes.


 The setter/getter convention requires getters to return autoreleased objects.

Not so, it requires that the returned object is not owned by the caller. That 
is all.


 So leaving it named according to the getter naming convention is wrong. 
 Adding new would not only be correct but would also warn against using it 
 as a getter. 

But it is a getter, no more or less. The lazy initialization is neither here 
nor there to the caller.

 As written (no matter what it is named), it is in fact the caller's job to 
 release it. That is presumably why you release it in -dealloc. If your 
 class's -init method initially created the first object assigned to the iVar, 
 whether by calling -eventTypes or by directly calling -newEventTypes and 
 setting the iVar, then you are the owner and you must release it in -dealloc 
 if not earlier. And if you call -eventTypes or -newEventTypes while 
 substituting a new object for the old iVar object in midstream, you must 
 release the old iVar object in midstream, too.


The object owns mEventTypes, so it is not the caller's job to release it. It is 
released in -dealloc because the object owns mEventTypes.

 At it's core, that's what is really odd about the code. It uses a getter 
 that does not comply with the getter convention.

I believe it does. I think your reading of the getter convention may be 
incorrect. If you can point to explicit documentation that states that the 
returned object must belong to an autorelease pool, I'll stand corrected, but 
that would be the first time I've ever heard that in 13 years of Cocoa 
programming!

--Graham




___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

On Sep 12, 2013, at 11:52 AM, Graham Cox graham@bigpond.com wrote:

At it's core, that's what is really odd about the code. It uses a getter
that does not comply with the getter convention.

I believe it does. I think your reading of the getter convention may be
incorrect. If you can point to explicit documentation that states that the
returned object must belong to an autorelease pool, I'll stand corrected, but
that would be the first time I've ever heard that in 13 years of Cocoa
programming!

I participated in a lengthy online debate about this very subject in 2002 with
Apple's Ali Ozer and a bunch of other folks whose names are very familiar on
this list. Opinions were all over the map. Eventually, as I recall, Ali Ozer
won the debate by issuing a technote or QA about the proper way to write
getters and setters. I just spent a little time looking for it but have not yet
found it. As I recall, it stated that the return [[x retain] autorelease]
pattern is preferred for getters and gave many reasons for preferring it. I
understand that @synchronize generates getters that comply with this
preference, and I also recall that some of the early documentation about
properties discussed this convention.

I did find a message from Ali dated July 31, 2002, at
http://www.cocoabuilder.com/archive/cocoa/50627-accessor-methods-and-auto-release-memory-trail.html
in which he states the autorelease preference. His message also said that a
faster alternative when speed is paramount is to return the variable without
retain or autorelease, and the code example he gave implied that the variable
was retained (it was retained or copied in the setter and returned in the
getter without modification). So I have to qualify what I said earlier today by
noting that, in appropriate circumstances, getters can return retained objects
instead of autoreleased objects, although the latter is preferred. The point
remains, however, that the getter that started this thread returned an object
retained, and the caller therefore owns it and is responsible for releasing it.

By the way, anybody interested in reading Apple's autorelease pool patents can
go to the US Patent and Trademark Office Web site http://patft.uspto.gov and
search for patents nos. 5,687,370, 6,026,415, and 6,304,884. Ali Ozer is listed
as one of the inventors. Some of the historical and technical information in
the patents is really interesting.

The current statement of the Cocoa ownership rule is here:
https://developer.apple.com/library/ios/documentation/cocoa/conceptual/MemoryMgmt/Articles/mmRules.html.
It notes at the end that the rule is similar to but a little different from
the Core Foundation Create Rule. That, and the text of the Cocoa document,
suggest that my assertion that the -eventTypes method we are discussing must be
named with new is overstated because the Cocoa rule is different from the
Create Rule. The Cocoa Rule says that you do own an object returned by a method
having “alloc”, “new”, “copy”, or “mutableCopy” in its name, but it also spells
out several other cases where methods without those names still make the caller
the owner -- namely, by retaining it. Since the -eventTypes method does retain
the object, the Cocoa document's statement of the Cocoa rule confirms that the
caller of -eventTypes is in fact the owner of the object and responsible for
releasing it. If the behavior of Analyze in this connection does still in fact
change depending on whether you include new or similar terms in the method
name, consistently with at least part of the Cocoa rule, one proper way to make
the ownership of the object returned by -eventTypes clear would be to put new
in its name. Alternatively, apparently, you could continue to name it like a
getter and simply document the method to indicate ownership. Either way,
Analyze should not base its error reporting on the name of the method alone,
because it is the Create Rule, not the Cocoa rule, that relies solely on names.
So I am now once again puzzled by my discovery a couple of years ago that
adding new to Cocoa method names removed the error reports. It appears that
Analyze is enforcing a slightly different rule from the Cocoa rule, one that is
more consistent with the Core Foundation Create Rule.

Bill Cheeseman - b...@cheeseman.name

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?

2013-09-12 Thread Greg Parker

On Sep 12, 2013, at 1:45 PM, Lee Ann Rucker lruc...@vmware.com wrote:
 On Sep 12, 2013, at 9:57 AM, Bill Cheeseman wrote:
 As I recall, it stated that the return [[x retain] autorelease] pattern is 
 preferred for getters and gave many reasons for preferring it. 
 
 Coincidentally enough, we've just hit a case where that's bad - it's a 
 complicated situation with ObjC++ and C++ object lifetimes, resulting in an 
 accessor that uses that pattern being called as a result of that object being 
 in dealloc. By the time the autorelease pool pops the object it's trying to 
 release is long gone. It's KVO unregistering, so there's not much we can do 
 about it other than be smarter in our accessors.

In some cases you can add an @autoreleasepool inside the object's -dealloc 
method to catch these.


-- 
Greg Parker gpar...@apple.com Runtime Wrangler



___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?


On Sep 12, 2013, at 9:16 AM, Roland King r...@rols.org wrote:

 setEventTypes: takes a parameter and returns nothing. What it does with that 
 parameter is entirely up to it, it may retain it if it wishes to be released 
 later, or do any other thing it wants. If the analyser doesn't much like what 
 setEventTypes: is doing, it would complain about setEventTypes:, which it's 
 not doing, it's complaining about a caller. That method doesn't return 
 anything and has no special naming so the analyser will presume it correctly 
 to do nothing net-net to its argument. 
 
 -eventTypes doesn't need the new or copy because it does not return an object 
 owned by the caller. Whether the object it returns is already owned by itself 
 to be released later is irrelevant, the refcount has not been increased 
 before return to the caller so the method is correctly named and the analyser 
 can properly assume it to be a +0 method. And apart from that it's not even 
 called in the piece of code in question so that's not really relevant.


I don't follow you. -setEventTypes: is properly named and properly written. It 
is (except for the last statement) standard setter code. One would not expect 
an error here. It increases the refcount of the newly created object passed in 
the parameter and assigned to the iVar by calling -retain on it. The object 
that is released in -setEventTypes: is a different object, the old iVar. For 
code that refers to an iVar without regard to what object is assigned to it, 
you are correct that the net effect on the iVar's refcount is 0. This is a 
great convenience for most code. But for unusual code like the example given, 
where a new object is being created to replace the object currently assigned to 
iVar, care must be taken to distinguish between the different objects.

-eventTypes creates the new object with a refcount of 1 by calling 
-newEventTypes, and then it increments its refcount by 1 by calling 
-setEventTypes:. The new object now has a refcount of 2. The release of the new 
object at the end of -eventTypes drops it back to 1. That is 1 more than it was 
before -eventTypes was called, because the new object didn't exist before 
-eventTypes was called.

-eventTypes: is, in fact, a classic new method, though a bit oddly written. 
All in the one method (by calls to utility methods), it creates a new object 
with a refcount of 1 by calling +alloc indirectly, increases its refcount by 1 
more by calling -setEventTypes:, decreases its refcount by 1 to balance the 
second increase, and returns it with a refcount of 1 without autoreleasing it. 
It does own the new object, and the method should have new in its name.

The fact that -setEventTypes: releases the old object that was formerly held by 
the iVar isn't relevant to the Analyzer. It's the new object created in 
-eventTypes by the call to -newEventTypes that the Analyzer cares about. It is 
doing nothing more nor less than enforcing a naming convention. Fix the name 
and the Analyzer is happy. No need to change the code, because the code is 
correct.

-- 

Bill Cheeseman - b...@cheeseman.name

___

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Re: Analyser reports memory leak… where?