Skip to content

Commit

Permalink
fix leak spotted by Larry Campbell
Browse files Browse the repository at this point in the history
  • Loading branch information
@rfm
rfm committed Jan 5, 2024
1 parent 992377c commit db90ab6
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion Source/NSNotificationCenter.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ - (id) addObserverForName: (NSString *)name
name: name
object: object];

return observer;
return AUTORELEASE(observer);
}

/**
Expand Down

2 comments on commit db90ab6

@triplef
Copy link
Member

@triplef triplef commented on db90ab6 Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change causes our app to crash on launch.

I believe the change itself is correct, but as stated by Apple’s documentation observer must be retained elsewhere by NSNotificationCenter until the registration is removed.

Should this be done here?

libs-base/Source/NSNotificationCenter.m

Line 294 in db90ab6

obs->observer = o;

@rfm
Copy link
Contributor Author

@rfm rfm commented on db90ab6 Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In NSNotificationCenter the observer is not retained and the calling code is responsible for removing it before it is deallocated, so I didn't realise this method was documented to behave differently.

In which case I think the original code was correct, but needs new code to release the observer when it is removed from the notification center.

eg. at the end of -removeObserver:name:object: something like
if ([observer isKindOfClass: [GSNotificationObserver class]) RELEASE(observer);
or some more efficient mechanism.

Please sign in to comment.