Re: Issues with nextKeyView chain

[Wolfgang Lux]

Doug Simons wrote:

> Our application had a crash when deallocating a clipView, because  
> its nextKeyView pointed to an object that had already been freed.  
> On carefully reviewing the code, I discovered that the system is  
> designed such that each view can have _multiple_ next and previous  
> key views.
>
> I understand why a view could have multiple _previous_ key views  
> (since it's possible to have several other views all pointing to it  
> as their nextKeyView). But I don't believe a view should ever have  
> more than one _next_ key view.
>
> I've checked in a change to NSView.m (r30088) that fixes this  
> problem (part of the issue for us turned out to be caused by ivars  
> being zeroed out at the wrong point during nib loading).
>
> But it appears to me that the code could be cleaned up and  
> simplified quite a bit if everyone agrees with my assertion that a  
> view should only have one (or zero) nextKeyView. To begin with, the  
> _nextKeyView ivar could be turned into a direct pointer to the next  
> key view rather than a GSIArray. Also, the code is not well  
> encapsulated right now, as it makes direct changes to ivars of  
> other objects.
>
> Does anyone see any reason not to change nextKeyView to a single  
> object? Would someone familiar with that code like to make the  
> change? (In particular, I'm unsure whether there may be other  
> classes besides NSView that might be impacted…)

I think you are misunderstanding the code here. Each view already has  
at most one next and previous key view, which is saved in the first  
element of the respective arrays. The remaining elements of the  
arrays are used to keep track of those views that refer to this view  
as their next and previous key view, respectively. For instance,  
given the statements
  [P1 setNextKeyView: V];
  [P2 setNextKeyView: V];
  [P3 setNextKeyView: V];
  [V setNextKeyView: N1];
  [V setNextKeyView: N2];
  [V setNextKeyView: N3];
you will end up with the following situation (sorry for the poor  
ASCII art, you'll need a fixed pitch font to view this correctly):

                       +-- nextKV ->---\
     P1 -- nextKV ->-+ | +-<- prevKV -- N3
                      \|/
     P2 -- nextKV ->-- V --<- prevKV -- N2
                      /|\
     P3 -- nextKV ->-+ | +-<- prevKV -- N1
        \--<- prevKV --+

Note that the previous key view of V is P3 and the next key view is  
N3. The pointers to P1, P2, N1, and N2 must be kept in the previous  
key and next key view arrays so that the nextKV pointers of P1, P2,  
and P3 and the prevKV pointers of N1, N2, and N3 can be reset when V  
is deallocated, which is necessary to prevent crashes of the sort you  
were experiencing.

Wolfgang