Bug 46340

Summary: crash: saving a resized image
Product: LibreOffice Reporter: alex <devkral>
Component: PresentationAssignee: Michael Stahl <mstahl>
Status: RESOLVED FIXED QA Contact:
Severity: critical    
Priority: medium CC: sergio.callegari, serval2412
Version: 3.5.0 releaseKeywords: regression
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard: target:3.6.0 target:3.5.2
Attachments: backtrace gdb by soffice
bt 3.5 branch with symbols
bt master with symbols

Description alex 2012-02-20 06:07:44 UTC
Created attachment 57318 [details]
backtrace gdb by soffice

Steps to reproduce:
1. create a presentation
2. Insert an image
3. Resize it
4. right-click on picture and "save picture" (same image format as the image you use)
5. save
-> works
but if you restart libreoffice and repeat the last two steps (saving the image)
-> crash and an empty picture (0 bytes)

additional information: saving in a different format as the picture works

I think the problem is that libreoffice tries to reuse the existing picture but doesn't notice that the picture has changed.


Tested image formats: jpeg and png
Distribution: archlinux
Comment 1 Julien Nabet 2012-02-23 15:27:31 UTC
Created attachment 57559 [details]
bt 3.5 branch with symbols

I reproduced the bug on 3.5 branch (Debian x86-64) and attached bt with symbols.
I must finish to recompile master (future 3.6) to see if this bug is still there
Comment 2 Julien Nabet 2012-02-24 15:03:06 UTC
Created attachment 57612 [details]
bt master with symbols

I reproduced this bug on master and attached bt with symbols.
Comment 3 Julien Nabet 2012-02-26 09:28:56 UTC
*** Bug 46615 has been marked as a duplicate of this bug. ***
Comment 4 Michael Meeks 2012-02-29 05:50:23 UTC
breaking in __cxa_throw just before we ended up in this unwind situation I get (vs. master):

#0  0xb60106d0 in __cxa_throw () from /usr/lib/libstdc++.so.6
#1  0xae89eedb in OInputCompStream::readBytes (this=0x97cd738, aData=uno::Sequence of length 32768 = {...}, nBytesToRead=32768)
    at /data/opt/libreoffice/master/package/source/xstor/ocompinstream.cxx:134
#2  0xb71ba6d9 in fileaccess::shell::write (this=0x87a86a0, CommandId=426, aUnqPath="file:///tmp/reallynotthere.png", OverWrite=1 '\001', 
    aInputStream=...) at /data/opt/libreoffice/master/ucb/source/ucp/file/shell.cxx:1862
#3  0xb71a34b0 in fileaccess::BaseContent::insert (this=0x97b3080, nMyCommandIdentifier=426, aInsertArgument=...)
    at /data/opt/libreoffice/master/ucb/source/ucp/file/bc.cxx:1171
#4  0xb71a400c in fileaccess::BaseContent::execute (this=0x97b3080, aCommand=..., CommandId=426, Environment=...)
    at /data/opt/libreoffice/master/ucb/source/ucp/file/bc.cxx:416
#5  0xb58771f3 in ucbhelper::Content_Impl::executeCommand (this=0x977e648, rCommand=...)
    at /data/opt/libreoffice/master/ucbhelper/source/client/content.cxx:1558
#6  0xb5877370 in ucbhelper::Content::writeStream (this=0xbfffdb3c, rStream=..., bReplaceExisting=1 '\001')
    at /data/opt/libreoffice/master/ucbhelper/source/client/content.cxx:1048
#7  0xae864b9d in io_FileAccess::OFileAccess::writeFile (this=0x97cd538, FileURL="file:///tmp/reallynotthere.png", data=...)
    at /data/opt/libreoffice/master/fileaccess/source/FileAccess.cxx:783
#8  0xaf4056ed in SdGRFFilter::SaveGraphic (xShape=...) at /data/opt/libreoffice/master/sd/source/filter/grf/sdgrffilter.cxx:528
#9  0xaf5bb1bd in sd::DrawViewShell::FuTemporary (this=0x8e2e680, rReq=...) at /data/opt/libreoffice/master/sd/source/ui/view/drviews2.cxx:941
#10 0xaf5debe4 in SfxStubDrawViewShellFuTemporary (pShell=0x8e2e680, rReq=...)
    at /data/opt/libreoffice/master/workdir/unxlngi6.pro/SdiTarget/sd/sdi/sdslots.hxx:1134

ie.

#1  0xae89eedb in OInputCompStream::readBytes (this=0x97cd738, aData=uno::Sequence of length 32768 = {...}, nBytesToRead=32768)
    at /data/opt/libreoffice/master/package/source/xstor/ocompinstream.cxx:134
134	        throw lang::DisposedException();
(gdb) l
129	{
130	    ::osl::MutexGuard aGuard( m_rMutexRef->GetMutex() );
131	    if ( m_bDisposed )
132	    {
133	        ::package::StaticAddLog( ::rtl::OUString( RTL_CONSTASCII_USTRINGPARAM( OSL_LOG_PREFIX "Disposed!" ) ) );
134	        throw lang::DisposedException();
135	    }

Which is fun - the stream that the image is coming from, is prematurely disposed (somehow).
Comment 5 Michael Meeks 2012-02-29 06:03:24 UTC
Argh - this comes from the (cretinous) UNO stream implementation nightmare that hurts us in spades everywhere; the dispose we get comes from:

commit fd95f1ab6220c6a530fd2e4e727417f504a5db51
Author: Michael Stahl <mstahl@redhat.com>
Date:   Fri Dec 2 23:43:23 2011 +0100

    refactor SdrModel::GetDocumentStream
    
    Remove 3 ~identical implementations of GetDocumentStream and the associated
    struct SdrDocumentStreamInfo.

and is a side-effect of the LifecycleProxy not having a long enough scope - which itself is a problem since we can't make it any wider since the stream pops out through an UNO property ... the trace is appended.

Michael - any chance you can have a look ? and/or I wonder where else this bites us.

#0  OInputCompStream::dispose (this=0x958dfe8) at /data/opt/libreoffice/master/package/source/xstor/ocompinstream.cxx:303
#1  0xae0ce7b3 in OStorage::InternalDispose (this=0x953e798, bNotifyImpl=1 '\001')
    at /data/opt/libreoffice/master/package/source/xstor/xstorage.cxx:2050
#2  0xae0cea21 in OStorage::dispose (this=0x953e798) at /data/opt/libreoffice/master/package/source/xstor/xstorage.cxx:4599
#3  0xae0cecf9 in OStorage::~OStorage (this=0x953e798, __in_chrg=<optimized out>)
    at /data/opt/libreoffice/master/package/source/xstor/xstorage.cxx:1983
#4  0xae0cee50 in OStorage::~OStorage (this=0x953e798, __in_chrg=<optimized out>)
    at /data/opt/libreoffice/master/package/source/xstor/xstorage.cxx:2009
#5  0xb5b2a77d in cppu::OWeakObject::release() () from /data/opt/OOInstall/program/../ure-link/lib/libuno_cppuhelpergcc3.so.3
#6  0xae0b9c74 in OStorage::release (this=0x953e798) at /data/opt/libreoffice/master/package/source/xstor/xstorage.cxx:2322
#7  0xb5be7e9c in com::sun::star::uno::Reference<com::sun::star::embed::XStorage>::~Reference (this=0x95ab044, __in_chrg=<optimized out>)
    at /data/opt/libreoffice/master/solver/unxlngi6.pro/inc/com/sun/star/uno/Reference.hxx:117
#8  0xb5c272a5 in _Destroy<com::sun::star::uno::Reference<com::sun::star::embed::XStorage> > (__pointer=0x95ab044)
    at /usr/include/c++/4.6/bits/stl_construct.h:94
#9  __destroy<com::sun::star::uno::Reference<com::sun::star::embed::XStorage>*> (__last=<optimized out>, __first=<optimized out>)
    at /usr/include/c++/4.6/bits/stl_construct.h:104
#10 _Destroy<com::sun::star::uno::Reference<com::sun::star::embed::XStorage>*> (__last=<optimized out>, __first=<optimized out>)
    at /usr/include/c++/4.6/bits/stl_construct.h:127
#11 _Destroy<com::sun::star::uno::Reference<com::sun::star::embed::XStorage>*, com::sun::star::uno::Reference<com::sun::star::embed::XStorage> >
    (__last=0x95ab048, __first=<optimized out>) at /usr/include/c++/4.6/bits/stl_construct.h:153
#12 ~vector (this=0x94ea1f0, __in_chrg=<optimized out>) at /usr/include/c++/4.6/bits/stl_vector.h:350
#13 ~Impl (this=0x94ea1f0, __in_chrg=<optimized out>) at /data/opt/libreoffice/master/comphelper/source/misc/storagehelper.cxx:549
#14 checked_delete<comphelper::LifecycleProxy::Impl> (x=0x94ea1f0)
    at /data/opt/libreoffice/master/solver/unxlngi6.pro/inc/boost/checked_delete.hpp:34
#15 ~scoped_ptr (this=0xbfffd8ac, __in_chrg=<optimized out>)
    at /data/opt/libreoffice/master/solver/unxlngi6.pro/inc/boost/smart_ptr/scoped_ptr.hpp:80
#16 comphelper::LifecycleProxy::~LifecycleProxy (this=0xbfffd8ac, __in_chrg=<optimized out>)
    at /data/opt/libreoffice/master/comphelper/source/misc/storagehelper.cxx:553
#17 0xb6f01614 in SdrGrafObj::getInputStream (this=0x9351cc0) at /data/opt/libreoffice/master/svx/source/svdraw/svdograf.cxx:1335

        // can be loaded from the original document stream later
        if( pGraphic->HasUserData() )
        {
            ::comphelper::LifecycleProxy proxy;
            xStream.set(
                pModel->GetDocumentStream(pGraphic->GetUserData(), proxy));
        }

#18 0xb6fa74fa in SvxGraphicObject::getPropertyValueImpl (this=0x8b2ee50, rName="GraphicStream", pProperty=0x93513f8, rValue=empty uno::Any)
    at /data/opt/libreoffice/master/svx/source/unodraw/unoshap2.cxx:1697
#19 0xb6fb62c3 in SvxShape::_getPropertyValue (this=0x8b2ee50, PropertyName="GraphicStream")
    at /data/opt/libreoffice/master/svx/source/unodraw/unoshape.cxx:1835
#20 0xaed7de5a in SdXShape::getPropertyValue (this=0x8a0fd90, PropertyName="GraphicStream")
    at /data/opt/libreoffice/master/sd/source/ui/unoidl/unoobj.cxx:849
#21 0xb6fb64a0 in SvxShape::getPropertyValue (this=0x8b2ee50, PropertyName="GraphicStream")
    at /data/opt/libreoffice/master/svx/source/unodraw/unoshape.cxx:1815
#22 0xaec055d6 in SdGRFFilter::SaveGraphic (xShape=...) at /data/opt/libreoffice/master/sd/source/filter/grf/sdgrffilter.cxx:523
#23 0xaedbb1bd in sd::DrawViewShell::FuTemporary (this=0x933d268, rReq=...) at /data/opt/libreoffice/master/sd/source/ui/view/drviews2.cxx:941
#24 0xaeddebe4 in SfxStubDrawViewShellFuTemporary (pShell=0x933d268, rReq=...)
    at /data/opt/libreoffice/master/workdir/unxlngi6.pro/SdiTarget/sd/sdi/sdslots.hxx:1134
Comment 6 Not Assigned 2012-02-29 13:41:14 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0c6d89941118368ccbb596362a545db5d3a07dbe

fdo#46340: fix crash in SdrGrafObj::getInputStream:
Comment 7 Michael Stahl 2012-02-29 13:52:06 UTC
it is easy to see why it crashes, the mystery is really why it didn't crash before, as the old code used SdrDocumentStreamInfo instead of LifecycleProxy, which has the exact same problem of going out of scope.

before fd95f1ab6220c6a530fd2e4e727417f504a5db51 it used to work this way: the getInputStream would call one of 3 duplicated GetDocumentStream methods, which would then create an XInputStream and wrap it in a SvStream.  then getInputStream would wrap that SvStream again in an XInputStream.

during the utl::UcbStreamHelper::CreateStream the UcbLockBytes::setInputStream_Impl is called, which determines that the XInputStream is not seekable, and thus decides to copy the stream into a temp file, and thus the code before my refactoring survived the fact that the XStorage where the original XInputStream was taken from went out of scope, invalidating said XInputStream.
Comment 8 Not Assigned 2012-02-29 21:35:55 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-3-5":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=63a480aac59d459e9f759c5a346bebdda0c78acd&g=libreoffice-3-5

fdo#46340: fix crash in SdrGrafObj::getInputStream:


It will be available in LibreOffice 3.5.2.