views:

48

answers:

1

Hello everyone,

I'm trying to write this down as concisely as possible, but it's not easy to describe -- so thanks for reading =)

I'm the main developer of the Open Source iPhone Framework Sparrow. Sparrow is modeled after the Flash AS3 Library, and thus has an event system just like AS3. Currently, that system works by specifying selectors - but I would love to expand that system by allowing the use of blocks for event listeners. However, I'm stumbling over memory management issues.

I will show you a typical use-case of events - as they are handled now.

// init-method of a display object, inheriting from 
// the base event dispatcher class
- (id)init
{
    if (self = [super init])
    {
        // the method 'addEventListener...' is defined in the base class
        [self addEventListener:@selector(onAddedToStage:)
                      atObject:self
                       forType:SP_EVENT_TYPE_ADDED_TO_STAGE];
    }
    return self;
}

// the corresponding event listener
- (void)onAddedToStage:(SPEvent *)event
{
    [self startAnimations]; // call some method of self
}

That's quite straight-forward: When an object is added to the display list, it receives an event. Currently, the base class records the event listeners in an array of NSInvocation-objects. The NSInvocation is created in a way that it does not retain its target and arguments. (The user can make it do that, but in 99% of the cases, it's not necessary).

That these objects are not retained was a conscious choice: otherwise, the code above would cause a memory leek, even if the user removed the event listener in the dealloc-method! Here is why:

- (id)init
{
    if (self = [super init])
    {
        // [self addEventListener: ...] would somehow cause:
        [self retain]; // (A)
    }
    return self;
}

// the corresponding event listener
- (void)dealloc
{
    // [self removeEventListener...] would cause:
    [self release]; // (B)
    [super dealloc];
}

On first sight, that seems fine: the retain in the init-method is paired by a release in the dealloc method. However, that does not work, since the dealloc method will never be called, because the retain count never reaches zero!

As I said, the 'addEventListener...'-method does, for exactly this reason, not retain anything in its default version. Because of the way events work (they are almost always dispatched by 'self' or child objects, which are retained anyway), that is not a problem.

However, and now we come to the central part of the question: I cannot do that with blocks. Look at the block-variant of event handling, as I would like it to have:

- (id)init
{
    if (self = [super init])
    {
        [self addEventListenerForType:ADDED_TO_STAGE block:^(SPEvent *event)
        {
            [self startAnimations];
        }];
    }
    return self;
}

That looks great and would be very easy to use. However: when the user calls a method on 'self' or uses a member variable within the block -- which will be, well, almost always -- the block will automatically retain 'self', and the object will never be dealloc'ed.

Now, I know that any user could rectify this by making a __block reference to self, like this:

__block id blockSelf = self;
[self addEventListenerForType:ADDED_TO_STAGE block:^(SPEvent *event)
{
    [blockSelf startAnimations];
}];

But, honestly, I am sure almost all of the users would not know to do so or forget to do so. An API should be not only easy to use, but also hard to misuse, and this clearly violates that principle. Users of the API would most definitely misuse it.

What bugs me is that I know that 'self' does not have to be retained -- it works in my current implementation without retaining it. So I want to tell the block that he does not need to retain self -- me, the library, should tell the block that, so that the user does not have to think about it.

In my research, I have not found a way to do so. And I can't think of a way to change my architecture to fit that limitation of blocks.

Has anybody got an idea what I could do about it?
Even if you have not, thanks for reading this far -- I know it was a verbose question ;-)

+1  A: 

I think the design is fundamentally flawed, blocks or no blocks. You are adding objects as event listeners before they are fully initialised. Similarly, the object could still receive events while it is being dealloced. The adding and removing of event listeners should be decoupled from allocation/deallocation IMO.

As for your immediate issue, you may not need to worry about it. You are having a problem because you are implicitly adding a reference to an object to itself (indirectly) by referencing it in the block. Bear in mind that anybody else supplying a block is unlikely to be referencing the object that generates the event or its instance variables because they won't be in scope where the block is defined. If they are (in a subclass, for instance), there's nothing you can do except document the issue.

One way to reduce the likelihood of a problem is to supply the object generating the event as a property on your SPEvent parameter and use that in the block instead of referencing self. You'll need to do that anyway because a block need not be created in a context where the object generating the event is in scope.

JeremyP
Thanks a lot for your answer, Jeremy! Concerning the design: IMO there's nothing wrong in creating an event listener in the init method, as long as they listen to events from 'self' or child objects. And that's the case here. But anyway: the problem is that it will be used the way you mention above: in the init-method of a subclass. Thus, users will have access to self and will cause the problem. The SPEvent object does already have that parameter. I just fear that users will write code as the one above - and if that is not solvable from within the library, I'd rather leave blocks out at all.
PrimaryFeather
You must document the potential leak then. There's nothing you can do to stop them from referencing self or instance variables of self and the retaining of self by the block is documented by Apple.
JeremyP
What a pity :-( Anyway, thanks again for your time! Honestly, in that case, I think one cannot recommend using blocks that need to be retained for some time on a platform without garbage collection. Throw-away-blocks, as they are used in NSArray, etc., are fine, but all else is just too dangerous IMHO. It's just too easy to overlook. -- I'll not mark the question as answered right away, perhaps another user has another idea; I hope you're OK with that!
PrimaryFeather