[Libreoffice] [REVIEW][3-5] Prevent excessvie references to formula result tokens

Eike Rathke erack at redhat.com
Tue Jan 31 05:27:31 PST 2012

Hi Kohei,

On Monday, 2012-01-30 16:35:46 -0500, Kohei Yoshida wrote:

> I'd like
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=e2b11f4fd79dce4116badb0ecf6477546ca5d0d4
> cherry-picked to the 3-5 branch.  It is probably too late for change
> like this to be in the 3-5-0 branch so I won't even try.

I'd like to ask for an improvement ;)  see below.

> To reproduce the problem, open the following file in Calc
> http://people.freedesktop.org/~kohei/test-formula-fill-crash.ods
> then
> 1. select B1:B65536 (or just hit ctrl-shift-up)
> 2. fill down (or ctrl-D if you use the default key binding).
> That will currently either crash, or do a totally wrong thing.  If it
> doesn't crash, try undo and redo and it will eventually crash.
> The reason is that, during the fill, the formula token instance inside
> ScFormulaResult gets "copied" i.e. it re-uses the existing instance and
> increases its reference counter by one.  The problem is, this counter is
> unsigned 16-bit integer, and as soon as it goes above 65535 it rolls
> back to zero, and eventually the token instance gets deleted
> prematurely.

Argh, yes, right.

> The above change ensures that the formula result is cleared after each
> formula cell instance gets copied.  We don't need to copy the formula
> result during fill because they get re-interpreted once the copying is
> complete.

True. Though the patch fixes this only for the Fill case, the same crash
happens when copying B1 to clipboard, selecting B2:B65536 and paste.
That needs to be handled similar. Probably best would be to revert this
commit and change the ScFormulaResult copy-ctor to create a clone of the
token instead so one doesn't need to bother with it at various places.

> As an aside, although it's not necessary for this fix, on master we
> should probably use unsigned 32-bit integer to store reference counter
> for this just to future-proof ourselves.  16-bit integer seems a bit too
> small for this purpose.

I'd not recommend using 32-bit for the token reference count, that would
significantly increase memory footprint of every formula. Single tokens
weren't meant to be shared across formula cells.


LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120131/f2602bc5/attachment.pgp>

More information about the LibreOffice mailing list