Re: [Libreoffice] [PATCH] ScGlobal::GetAutoFormat not always required to create fresh instance

On 01/09/2012 08:10 AM, Markus Mohrhard wrote:
> In my opinion we have two bugs here.. The ScGlobal::ppRscStrin is
> being deleted in the ScModule destructor which means that calc is no
> longer open. So the question is why do we delete ScAutoFormatObj after
> ScModule and the second one is that we are creating the object in the
> destructor.

The ScAutoFormatObj is still held remotely when 
Desktop::DeregisterServices disposes the URP connection via which the 
object is held.  That UNO objects are held longer than necessary is 
normal, as e.g. a Java process only releases them when it decides to 
garbage-collect its proxies for those objects.  I assume that ScGlobal 
is cleaned up also due to some disposing from within 
Desktop::DeregisterServices, but the order in which things are disposed 
here is indeterminate.  So it can happen that Calc UNO objects are 
disposed after ScGlobal has already been cleared.  (It is generally 
understood to be a bad idea to do anything---other than merely cleaning 
up by---from within disposing/destroying UNO objects.)

This is indeed a general problem, and it can potentially cause other 
problems besides the one we see with the crash at hand.

> And then there might be a simple solution to both problems at the same
> time. If possible we could add a variable to ScAutoformatObj storing a
> sheet::SpreadsheetDocument (the uno interface behind ScModule and
> therefore force the right destruction order. I did not test this idea
> but I did something similar for ScDatabaseRangesObj and
> ScDatabaseRangeObj to prevent the destruction of the
> ScDatabaseRangesObj before the ScDatabaseRangeObj.

Three points:

1  If ScAutoFormatObj requires an ScModule (which in turn controls 
ScGlobal?), then it would indeed look correct to keep a reference to an 
ScModule instance with every ScAutoFormatObj instance.  What one would 
potentially need to look out for is ring references.  (I know way to 
little about the Calc architecture to know whether this is actually 
relevant here.)

2  On the other hand, it might also be an error that ScAutoFormatObj 
still requires an ScModule/ScGlobal during destruction (see above), in 
which case the proper fix could be to fix ~ScAutoFormatObj (by somehow 
redesigning the class to do the saving earlier).  (And in which case 
point 1 might become irrelevant---if some other protocol already 
guarantees that the ScModule/ScGlobal is alive as long as an 
ScAutoFormatObj may need it---, or an orthogonal issue that could be 
fixed independently.)

3  And after all, the null-checking calls to GetAutoFormat would still 
be dubious and should be addressed.  (But again, that would become an 
orthogonal issue that could be fixed independently.)

Stephan