Re: [PATCH] Minor fix in NSWindowController

[Fred Kiefer]

Hi Quentin,

I am rather unsure about this patch.I don't quite remember, who did put
in these commetn lines on retaining the window, but I would expect that
there were reasons for it at that time. Still they may have been the now
redundant solution of an already resolved problem.
Did you do sufficent tests with your patch? As there aren't that many
document based applications for GNUstep, we really shoud try all of
them, before releasing the patch.

Cheers
Fred

Quentin Mathé wrote:
> 
> Here is a little patch to have proper window controller deallocation 
> when used in document architecture context. It is based on an initial 
> bug report by Saso Kiselkov.
> 
> I'm going to briefly summarize Cocoa documentation…
> When you are using a window controller within the document
> achitecture context (NSDocument + NSDocumentController), -
> -releasedWhenClosed value isn't taken in account : when a window is
> closed, the window is always released by the window controller,
> itself released by the document object. That's not true for a window
> controller used alone.
> 
> To enter in the details, in our case, the current NSWindowController
> code includes a dubious retain call on its window ivar (within
> _windowWillClose: private method). This prevents the window to be
> correctly deallocated unlike its window controller.
> 
> The -retain call currently presents code is unbalanced, that results  in
> a memory leak and sometimes in a segmentation fault.
> For example… You decide to use a toolbar with your document window  and
> the document object plays the role of the toolbar delegate. When  the
> document window is closed, the window controller and the document 
> object are released, but the document window is not (and its toolbar 
> neither).
> The next time you are going to open or create an identical document,  a
> new document window will be created with a fresh toolbar. When you  are
> going to manipulate this toolbar, it will try to propagate its  state to
> other toolbars still in use with the same identifier. At  this moment,
> the undeallocated toolbar will try to ask its delegate  for items, but
> the delegate (document object) has already been  deallocated, this
> invalid pointer access will then trigger a  segmentation fault.
> 
> The fix just consists to remove the -retain call, then everything is 
> properly deallocated on document window closing. Since I don't really 
> understand the explanation in the comment related to the offending 
> line), I'm perhaps missing an important detail in with this patch,  yet
> I have experienced no issue in my tests with this fix.