Re: Serious bug in NSTextView (and a fix)

[Nicola Pero]

Hi Ludovic,

sorry, am a bit ill today, so got sort of a fog in mind.

I quite trust your testings :-) but didn't understand all of the
explanation ... I would like to understand better why you get a crash at
all ... - is the problem happening because the notification is retaining
the NSTextView and causing it to be deallocated too late (say, after its
window) ? 

I suspect this because I notice that NSWindow -dealloc is not calling [_wv
viewWillMoveToWindow: nil] before releasing _wv - while I think it should,
because releasing _wv will not necessarily dealloc it, so we need to make
sure we clean out the pointer in the views to the NSWindow before
releasing _wv, otherwise _wv (and all subviews - which includes your
NSTextView) will be left with an invalid pointer to the window and will
likely crash at some point in the future. Can you try adding

[_wv viewWillMoveToWindow: nil];

in NSWindow.m -dealloc, before TEST_RELEASE (_wv), and let me know if we
got any luck with this.

It's probably wrong, I'm a bit ill today, sorry, but let me know if you
find out more about this problem.  Anyway, in my fogs I seem to think that
releasing the notification as in your patch is possibly only hiding the
real bug, which is somewhere else.

Comments/suggestions from other developers appreciated.

> When -dealloc is called on NSTextView, -dealloc is called on 
> NSText and on NSView.
> 
> In NSView -dealloc, we call NSView: -_unregisterDraggedTypes. This 
> method calls _removeDragTypes from 
> xgps/Source/SharedX/XGContextEvent.m.
> 
> The problem here is that we can have a crash in _removeDragTypes 
> since  "self" (our NSTextView object) can still be referenced by 
> an object in the autorelease pool and get released.
> 
> The object itself, is a NSNotification object created when we post
> five critical notifications those NSTextView's methods:
> 
> NSTextView: -setRulerVisible
> NSTextView: -shouldChangeTextInRange: replacementString
> NSTextView: -didChangeText
> NSTextView: -resignFirstResponder
> NSTextView: -_illegalMovement:
> 
> Actually, IMO, only -resignFirstResponder can be problematic. We
> can safely replace in NSTextView.m:
> 
> - (BOOL) resignFirstResponder
> {
>   ...
>   ...
>   [nc postNotificationName: NSTextDidEndEditingNotification  
>       object: _notifObject];
>   
>   return YES;
> }
> 
> by:
> 
> - (BOOL) resignFirstResponder
> {
>   NSNotification *aNotification;
>   ..
>   aNotification = [[NSNotification alloc]
> initWithName: NSTextDidEndEditingNotification
>                                         object: _notifObject
>                                         userInfo: nil];
>   [nc postNotification: aNotification];
>   RELEASE(aNotification);
> 
>   return YES;
> }
> 
> To be *sure* that our NSTextView object won't be retained by an object
> (a NSNotification object) in an autorelease pool.
> 
> I've done extensive testing for that with GNUMail.app and I can tell
> that it works and solve the problem.
> 
> Thanks a lot,
> 
> Ludo
> 
>