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. Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD
Attachment:
pgpuRer28MdMr.pgp
Description: PGP signature