Issue 84159

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() 	

so it's confirmed

Andreas, please have a look.
Whether this can be seen as a show stopper depends on your findings.

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

ama->fme: Please have a look.
Crash in SwFmtCharFmt::Modify(..), (*this) seems to be already destructed.
A lot of asserts from SwTxtNode::DestroyAttr(..).

fme: Crashes even OOo 1.x => no showstopper.

...so it's not broken ;-)
Adjusting internal flags.

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. 

Created attachment 50412 [details]
second bugdoc

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.

Cannot be fixed until code freeze => target 3.x

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.

Just want to confirm it's still an issue with 3.0.0-9284.

*** Issue 104896 has been marked as a duplicate of this issue. ***

Reassigned

This is sure to bite me, too, sooner or later.

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.

->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

>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 

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... :-)

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.

@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)

The crash occurs when the document is closed, not when it is saved... 

I would have noticed that ... if it happened.
It didn't.

Do you know which field/variable is overflow?

Where is located (please specify path)?

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.

Created attachment 70043 [details]
Using standard OOo version 3.2.1, crashes when document is closed.

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).

attachment id 70043 does indeed crash OOo.
So apparently the latest version that I could find on the website, is not the same?

This happens in the array of SfxItemPool_Impl:
http://svn.services.openoffice.org/opengrok/xref/DEV300_m80/svl/source/inc/poolio.hxx

Crash still happens on Windows 7 64-bit OpenOffice 3.2.1, just open and close 
AndrewMacro.odt

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

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.

That's great news! I guess it's not in OOO330m10 yet? Is there other way to test
it without compiling from sources?

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.

@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()

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.

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  :)

@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.

@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  :)

New 32bit constant SFX_ITEMS_DEFAULT was added:
http://hg.services.openoffice.org/cws/svarray/rev/a3ede68b5a05

Can we send this CWS to QA?

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.

Resolved in CWS svarray:
http://hg.services.openoffice.org/cws/svarray

@sba:
please verify

Verified in CWS svarray

Verified in OO.o 3.4 Beta. The same AndrewMacro doc that crashes 3.3 now opens and closes with no error.

Closing.

*** Issue 120325 has been marked as a duplicate of this issue. ***