Issue 84159

Summary: Crash while closing the document: SfxItemPool cannot contain more than 2^16 non-poolable items
Product: Writer Reporter: arysin <arysin>
Component: codeAssignee: stefan.baltzer
Status: CLOSED FIXED QA Contact: issues <issues.openoffice.org>
Severity: trivial    
Priority: P3 CC: andrew, bjoern.michaelsen, clippka, cno, gang65, issues, mst.ooo, mux2005, tj, williammosley
Version: OOo 2.3.1 RC1   
Target Milestone: 3.4.0   
Hardware: All   
OS: All   
Issue Type: DEFECT Latest Confirmation on: ---
Developer Difficulty: ---
Attachments:
Description Flags
second bugdoc
none
Using standard OOo version 3.2.1, crashes when document is closed. none

Description arysin 2007-12-02 01:58:34 UTC
This defect was there since at least OOo 2.2.
While reporting it with autoreport after Writer crash I did not get any reply so
I'll file it here.

I open the document and when I close it I consistently get a Writer crash. This
was the case for all writers at least from 2.2 and it's still an issue with
2.3.1RC1. I used official rpms and rpms from my distribution on Linux (Mandriva)
both in 32 and 64-bit environment. The same happens on Windows platform. The
document can be found here http://dict.linux.org.ua/other/sheludko.odt

I had this crash consistently with the big file I had for ~3000 pages and I
though it was related to the size. Now I have it for much smaller one, and it
looks related to the styles I use (Krym*).

It's basically making OpenOffice.org unusable for anybody who works with the
files like this one. So I hope this can be fixed in 2.3.1.
Comment 1 Mathias_Bauer 2007-12-02 10:36:40 UTC
Here's a call stack on WinXP with an m231 build:

 	sw680mi.dll!SwFmtCharFmt::Modify()  + 0xf bytes	C++
	sw680mi.dll!SwFmt::Modify()  + 0x184 bytes	C++
 	sw680mi.dll!SwFmt::~SwFmt()  + 0x9d bytes	C++
 	sw680mi.dll!SwCharFmt::`scalar deleting destructor'()  + 0x3c bytes	C++
 	sw680mi.dll!SwCharFmts::DeleteAndDestroy(unsigned short nP=0, unsigned short
nL=8)  Line 177 + 0xad bytes	C++
 	sw680mi.dll!SwCharFmts::~SwCharFmts()  + 0x21 bytes	C++
 	sw680mi.dll!SwCharFmts::`scalar deleting destructor'()  + 0xb bytes	C++
 	sw680mi.dll!SwDoc::~SwDoc()  + 0x54c bytes	C++
 	sw680mi.dll!SwDoc::`scalar deleting destructor'()  + 0xb bytes	C++
 	sw680mi.dll!SwDocShell::RemoveLink()  + 0x97 bytes	C++
 	sw680mi.dll!SwDocShell::~SwDocShell()  + 0x80 bytes	C++
 	sw680mi.dll!SwDocShell::`vbase destructor'()  + 0x8 bytes	C++
 	sw680mi.dll!SwDocShell::`vector deleting destructor'()  + 0x39 bytes	C++
 	tl680mi.dll!SvRefBase::QueryDelete()  + 0xd bytes	C++
 	sfx680mi.dll!05aeaa9e() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for
sfx680mi.dll]	
 	sfx680mi.dll!05ba0f95() 	
 	sfx680mi.dll!05b9cd86() 	
 	sfx680mi.dll!05b9f728() 	
 	sfx680mi.dll!05b9c25e() 	
 	sfx680mi.dll!05b59212() 	
 	sfx680mi.dll!05b1bb90() 	
 	fwk680mi.dll!framework::Frame::setComponent()  + 0x141 bytes	C++
 	fwk680mi.dll!framework::CloseDispatcher::implts_establishBackingMode()  +
0x15f bytes	C++
 	fwk680mi.dll!framework::CloseDispatcher::impl_asyncCallback()  + 0x269 bytes	C++
 	fwk680mi.dll!framework::CloseDispatcher::LinkStubimpl_asyncCallback()  + 0xe
bytes	C++
 	tl680mi.dll!Link::Call()  + 0x11 bytes	C++
 	vcl680mi.dll!vcl::EventPoster::DoEvent_Impl()  + 0x12 bytes	C++
 	vcl680mi.dll!vcl::EventPoster::LinkStubDoEvent_Impl()  + 0xe bytes	C++
 	tl680mi.dll!Link::Call()  + 0x11 bytes	C++
 	vcl680mi.dll!ImplHandleUserEvent(ImplSVEvent * pSVEvent=0x10b22370)  Line 2024	C++
 	vcl680mi.dll!ImplWindowFrameProc(void * pInst=0x032ae198, SalFrame *
__formal=0x032a8580, unsigned short nEvent=22, const void * pEvent=0x10b22370) 
Line 2521 + 0x9 bytes	C++
 	vcl680mi.dll!SalFrame::CallCallback()  + 0x16 bytes	C++
 	vcl680mi.dll!ImplHandleSalObjSysCharMsg()  + 0x642 bytes	C++
 	vcl680mi.dll!SalFrameWndProc()  + 0x698 bytes	C++
 	vcl680mi.dll!SalFrameWndProcW()  + 0x30 bytes	C++
 	user32.dll!77d48709() 	
 	user32.dll!77d487eb() 	
 	user32.dll!77d70494() 	
 	user32.dll!77d489a5() 	
 	user32.dll!77d493df() 	
 	user32.dll!77d70494() 	
 	user32.dll!77d489e8() 	
 	vcl680mi.dll!ImplDispatchMessage()  + 0x15 bytes	C++
 	vcl680mi.dll!WinSalInstance::AcquireYieldMutex()  + 0x36 bytes	C++
 	vcl680mi.dll!ImplSalYield()  + 0x45 bytes	C++
 	vcl680mi.dll!WinSalInstance::Yield()  + 0x9f bytes	C++
 	vcl680mi.dll!Application::Yield()  + 0x3a bytes	C++
 	vcl680mi.dll!Application::Execute()  + 0x24 bytes	C++
 	soffice.bin!desktop::Desktop::Main()  + 0x1041 bytes	C++
 	vcl680mi.dll!ImplSVMain()  + 0x64 bytes	C++
 	vcl680mi.dll!SVMain()  + 0x1c bytes	C++
 	soffice.bin!rtl::OUString::~OUString()  + 0x86 bytes	C++
 	soffice.bin!_WinMain@16()  + 0x15 bytes	C++
 	soffice.bin!__tmainCRTStartup()  Line 578 + 0x35 bytes	C
 	soffice.bin!WinMainCRTStartup()  Line 403	C
 	kernel32.dll!7c816d4f() 	
 	kernel32.dll!7c8399f3() 	
Comment 2 Mathias_Bauer 2007-12-02 10:37:23 UTC
so it's confirmed
Comment 3 Mathias_Bauer 2007-12-02 10:38:43 UTC
Andreas, please have a look.
Whether this can be seen as a show stopper depends on your findings.
Comment 4 max.odendahl 2007-12-02 10:44:31 UTC
During loading with non-pro, I also get:

Error: Ins 1 memtools\svarray.cxx line 77

and 

removing item not in pool with id /Pos 42 itempool.cxx line 913
Comment 5 andreas.martens 2007-12-03 08:04:35 UTC
ama->fme: Please have a look.
Crash in SwFmtCharFmt::Modify(..), (*this) seems to be already destructed.
A lot of asserts from SwTxtNode::DestroyAttr(..).
Comment 6 frank.meies 2007-12-03 08:31:06 UTC
fme: Crashes even OOo 1.x => no showstopper.
Comment 7 michael.ruess 2007-12-03 08:52:01 UTC
...so it's not broken ;-)
Adjusting internal flags.
Comment 8 frank.meies 2007-12-18 09:54:26 UTC
Open second attached bugdoc. There are 32761 paragraphs in the document, each of
them has two text portions using a character style (so there are 2 * 32761 =
65522 character style portions in the text). No crash on closing the document.
If you copy on of the paragraphs to the end of the document, you have 65524
character style text portions, which is quite close to 2^16. So I suspect this
bug is caused by an svarray overflow. 
Comment 9 frank.meies 2007-12-18 10:54:00 UTC
Created attachment 50412 [details]
second bugdoc
Comment 10 frank.meies 2007-12-18 13:16:10 UTC
Some further investigations reveal, that the SfxItemPool class cannot contain
more than 2^16 items of one type, because the underlying implementation uses the
good old svarray. Since the character format item is not poolable, the result is
that you cannot have more that ~ 2^16 character style text attributes, no matter
if there are 2^16 different character style text attributes or 2^16 times the
same character attribute.

This needs some refactoring, but cannot be fixed for 2.4. I'll adjust the target.
Comment 11 frank.meies 2008-01-10 11:45:32 UTC
Cannot be fixed until code freeze => target 3.x
Comment 12 arysin 2008-03-27 13:38:15 UTC
Now that 2.4 is out, I'd like to "ping" this issue so it does not "get lost". It
seems that the problem is in the framework used so the earlier it's handled the
better chance for the fix to get into 3.0 :)
Sorry for the noise but in my opinion this is really critical and also
preventing me from forcing all of my collegues to move to OOo.
Comment 13 arysin 2008-04-06 05:33:58 UTC
Just want to confirm it's still an issue with 3.0.0-9284.
Comment 14 Oliver Specht 2009-09-09 10:41:30 UTC
*** Issue 104896 has been marked as a duplicate of this issue. ***
Comment 15 Oliver Specht 2009-09-09 10:42:05 UTC
Reassigned
Comment 16 T. J. Frazier 2009-09-09 13:06:09 UTC
This is sure to bite me, too, sooner or later.
Comment 17 andrew 2009-09-09 13:51:06 UTC
Added myself to the CC list because 104896 was marked as a duplicate of this.
Seems that this bug now causes AndrewMacro.odt to crash.
Comment 18 Oliver Specht 2009-09-22 12:44:32 UTC
->b_michaelsen: It might make sense not only to replace the implementation of
SfxPooItemArray_Impl in svtools but to also think about making the SwFmtCharFmt
poolable.

Reassigned to b_michaelsen
Comment 19 henke54 2010-02-03 19:13:51 UTC
>Seems that this bug now causes AndrewMacro.odt to crash.
Yes, i must confirm this also ...
I had to 'hardrestart' my comp. and got then as 'crashmessage' :

 /usr/lib/openoffice/program/soffice  doesn't react

I am using ubuntu 9.10 / firefox 3.5.7 / GoOo 3.1.1 
Comment 20 andrew 2010-02-04 01:03:19 UTC
Yeah, I have a stack of changes that I have not added because my document
crashes. I have only run across a few documents that crash because of this
issue, but, when it does, you are done making changes... :-)
Comment 21 zzarko 2010-05-10 11:31:44 UTC
Just to confirm this bug still exists in OOo 3.2 (Ubuntu 9.10, Sun's OOO320m12
build 9483). Tested with Fme's charstylescrash.odt and Pitonyak's AndrewMacro.odt.
Comment 22 cno 2010-06-15 08:03:20 UTC
@pitonyak:
I look in AndrewMacro.odt now and then.
Just opened it, did a minor edit on a random location, and saved.
(page 493 - 23.121. RSet Statement
heading Syntax: added space)

No crash ...
(Ubuntu 10.04, OOoDEV300m82 and OOo 3.2.1  Dutch)

Comment 23 andrew 2010-06-16 13:44:00 UTC
The crash occurs when the document is closed, not when it is saved... 
Comment 24 cno 2010-06-16 13:54:47 UTC
I would have noticed that ... if it happened.
It didn't.
Comment 25 gang65 2010-06-16 14:34:42 UTC
Do you know which field/variable is overflow?

Where is located (please specify path)?
Comment 26 gang65 2010-06-16 15:18:57 UTC
In file:
http://svn.services.openoffice.org/opengrok/xref/DEV300_m82/svl/source/items/itempool.cxx

I found SV_IMPL_PTRARR( SfxPoolVersionArr_Impl, SfxPoolVersion_Impl* );
declaration.

Does this SvArray couse this trouble?

If yes I could prepare patch to solve it.
Comment 27 andrew 2010-06-17 02:10:56 UTC
Created attachment 70043 [details]
Using standard OOo version 3.2.1, crashes when document is closed.
Comment 28 andrew 2010-06-17 02:13:47 UTC
cornouws, can you try loading the latest version of AndrewMacro.odt that I
uploaded and then close the document (do not save it first). When I try using
the current 64-bit version on Fedora and the latest windows release on XP, there
is a crash on close (just load then close).
Comment 29 cno 2010-06-17 07:46:46 UTC
attachment id 70043 does indeed crash OOo.
So apparently the latest version that I could find on the website, is not the same?
Comment 30 gang65 2010-06-29 14:20:51 UTC
This happens in the array of SfxItemPool_Impl:
http://svn.services.openoffice.org/opengrok/xref/DEV300_m80/svl/source/inc/poolio.hxx
Comment 31 minfo 2010-07-03 20:11:08 UTC
Crash still happens on Windows 7 64-bit OpenOffice 3.2.1, just open and close 
AndrewMacro.odt
Comment 32 Risto Jääskeläinen 2010-07-12 06:26:34 UTC
Interestingly duplicate Issue 104896 reported by pitonyak is targeted 3.3 but
this is targeted 3.x which may be later than 3.3?

Anyhow in DEV300m82 on Vista shows this crash.

Regards 
Risto
Comment 33 gang65 2010-10-12 21:42:46 UTC
I think I solved this bug in the CWS svarray:
http://hg.services.openoffice.org/cws/svarray/summary

Commit which should solve this bug:
http://hg.services.openoffice.org/cws/svarray/rev/df7b4d57529a

Please check the solution is it works for you.
Comment 34 arysin 2010-10-13 13:07:21 UTC
That's great news! I guess it's not in OOO330m10 yet? Is there other way to test
it without compiling from sources?
Comment 35 bjoern.michaelsen 2010-10-13 14:17:42 UTC
Hi Bartosz,

I have not tested the stuff, the changes in your cws look like a good solution!
I see that cws svarray currently does not have a qa rep yet. Do you have anyone
at hand for that? How do you intend to proceed with the cws (e.g. do you still
want to do other work in it)? If you are pretty much finished, we would just
need to resync to a current milestone, build on the tinderboxes and then do some
testing to get it in the master.
Comment 36 mst.ooo 2010-10-13 16:57:56 UTC
@Bartosz:

the fix looks good, except for the bit where the size of the array is
stored into a stream.
according to the discussion on dev@openoffice.org, it should not be a
problem to simply change the stream format.
because this is a serialization we should use fixed size integer types.
size_t is not fixed size, it can be 32 or 64 bit depending on C++ implementation.

i think the following should work:

in SfxItemPool::Store() ~line230 initialize the count like this:
sal_uInt32 nCount = ::std::min<size_t>( (*pArr)->size(), SAL_MAX_UINT32 );

this way we never write more than 2^32 items.
should be enough even for Andrew :)

in SfxItemPool::Load() ~line635 just change type of count to sal_uInt32.
and adapt the signature of readTheItems()
Comment 37 gang65 2010-10-18 21:10:27 UTC
Next release of the fix is available:
http://hg.services.openoffice.org/cws/svarray/rev/94f924a805ad

I tested it on: sheludko.odt, AndrewMacro.odt and charstylescrash.odt documents.
The OpenOffice don't crash.

The most slow is charstylescrash.odt, so be patient.

Feel free to test it.
After successful check We could start the QA process.
Comment 38 mst.ooo 2010-10-19 11:37:18 UTC
while reviewing your commit i noticed that
there is one more thing that needs adapting (your FIXME comment was a hint):

this is from SfxItemPool::GetItem():
	if ( nOfst == SFX_ITEMS_STATICDEFAULT )
		return *(ppStaticDefaults + GetIndex_Impl(nWhich));

the constant SFX_ITEMS_STATICDEFAULT is compared with an index here;
the index is now a sal_uInt32.

WRONG IDEA (see below):
so the following constants also need to be 32 bit:

#define SFX_ITEMS_POOLDEFAULT               0xffff
#define SFX_ITEMS_STATICDEFAULT             0xfffe
#define SFX_ITEMS_DELETEONIDLE              0xfffd

and the nKind member of SfxPoolItem should be a sal_uInt32 (and the signatures
of GetKind()/SetKind() adapted).


no, wait, on second thought (with the help of opengrok)...
 this is not a good idea.
the _real_ problem here is that one of these values,
SFX_ITEMS_STATICDEFAULT, is used for 2 different things:
- as a Kind value of items
- as a index/surrogate in the item pool

these are _different_, and there really should be _different_ constants for them.

so, a better way would be to introduce a new 32-bit constant, maybe
SFX_ITEMS_DEFAULT, that should be used in the second case.

the places where this should be used are those where the constant
 is not used a s a Kind:
SfxItemPool::LoadSurrogate()
>	// dflt-Attribut?
>	if ( SFX_ITEMS_STATICDEFAULT == nSurrogat )

SfxItemPool::GetSurrogate()
>	// Pointer auf static- oder pool-dflt-Attribut?
>	if( IsStaticDefaultItem(pItem) || IsPoolDefaultItem(pItem) )
>		return SFX_ITEMS_STATICDEFAULT;

SfxItemPool::GetItem()
>	// dflt-Attribut?
>	if ( nOfst == SFX_ITEMS_STATICDEFAULT )

sc/source/ui/unoobj/defltuno.cxx:
> 380         const SfxPoolItem* pItem = pPool->GetItem( pEntry->nWID,
SFX_ITEMS_STATICDEFAULT );


Bartosz, please double-check this, and fix it  :)
Comment 39 bjoern.michaelsen 2010-10-19 14:22:21 UTC
@Bartosz,mst: I am assuming you want get the changes into the master with cws
svarray, right? I am asking because mst did review some of the changes as
patches. Those where not merged into an existing cws, right?

I am currently on vacation, but will help with the QA stuff ASAP when I return.
If you have urgent requests in the meantime please ask mst. I will be lurking
here, but on a vacation my response time might be subpar. ;)

As I do not see your name yet on:
http://www.openoffice.org/copyright/copyrightapproved.html
you might consider following the steps at:
http://wiki.services.openoffice.org/wiki/Sun_Contributor_Agreement
because that might take some time. Stefan Taxhet <st@openoffice.org> will help
you there if any questions arise.
Comment 40 mst.ooo 2010-10-19 15:07:57 UTC
@b_michaelsen:
yes, this work is in CWS svarray.
and Bartosz already did the paperwork, it's not his fault that the web page is
outdated  :)
Comment 41 gang65 2010-10-19 19:46:47 UTC
New 32bit constant SFX_ITEMS_DEFAULT was added:
http://hg.services.openoffice.org/cws/svarray/rev/a3ede68b5a05

Can we send this CWS to QA?
Comment 42 mst.ooo 2010-10-20 10:41:56 UTC
i've added the issue to the "svarray" CWS.

i'll assign it to Bartosz: please resolve it as "fixed".

then we can proceed with your CWS as described in the mail i sent you.
Comment 43 gang65 2010-10-20 14:31:40 UTC
Resolved in CWS svarray:
http://hg.services.openoffice.org/cws/svarray
Comment 44 mst.ooo 2010-10-25 12:03:27 UTC
@sba:
please verify
Comment 45 stefan.baltzer 2010-10-25 14:22:13 UTC
Verified in CWS svarray
Comment 46 T. J. Frazier 2012-04-19 11:12:38 UTC
Verified in OO.o 3.4 Beta. The same AndrewMacro doc that crashes 3.3 now opens and closes with no error.

Closing.
Comment 47 Ariel Constenla-Haile 2012-07-20 21:01:03 UTC
*** Issue 120325 has been marked as a duplicate of this issue. ***