Issue 105343 - I/O error and/or loop when saving a Draw document containing >2 shapes in binary fromat
Summary: I/O error and/or loop when saving a Draw document containing >2 shapes in bin...
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: code (show other issues)
Version: DEV300m59
Hardware: All All
: P2 Trivial (vote)
Target Milestone: OOo 3.2
Assignee: stefan.baltzer
QA Contact: issues@graphics
URL:
Keywords:
: 107309 (view as issue list)
Depends on:
Blocks: 99999
  Show dependency tree
 
Reported: 2009-09-24 22:38 UTC by eric.savary
Modified: 2010-01-12 13:17 UTC (History)
8 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
The document that is generated in case of the scenario with three shapes ( that seems to happen only on some machines ). (27.50 KB, application/octet-stream)
2009-11-16 19:58 UTC, mikhail.voytenko
no flags Details
The patch that solves the problem. (2.07 KB, text/plain)
2009-11-23 12:51 UTC, mikhail.voytenko
no flags Details
A simpler patch that solves the problem (1.54 KB, patch)
2009-11-23 13:31 UTC, matthias.huetsch
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description eric.savary 2009-09-24 22:38:57 UTC
Reproduced on XP and Vista (SBA, MRU, CD and ES machines but on some others not!)

- New Draw document
- insert 2 shapes (rectangles)
- Save as StarDraw 5.0 (*.sda)
- Reload
-> I/O error

With 3 shapes -> loop on reload.

Tests:
- Does not happen on every machine
- Happens only with Draw (especially not in Impress)
- must have been introduced around m56-m58 (not reproducible on m55)

Proposing as showstopper.
Comment 1 clippka 2009-09-25 08:40:43 UTC
taking over
Comment 2 clippka 2009-09-25 14:42:02 UTC
mav told me this may be a duplicate to issue 105082. I checked and it still
occours with this fix.

From the assertion I get during reload this looks like a broken stream, have to
investigate...
Comment 3 carsten.driesner 2009-09-25 14:46:57 UTC
cd: Set mav on CC.
Comment 4 clippka 2009-09-28 11:54:37 UTC
cl->mav: as you suggested I tried with a sal3.dll from m55 and it worked
Comment 5 mikhail.voytenko 2009-10-05 10:16:30 UTC
The problem looks not to be related to the sal implementation, the clipboard
listener uses SfxViewShell that is already deleted. The crash does not happen
always, sometimes the memory still contains partially valid data, that allows
office to survive. I suspect that the exchange of the sal library has just
affected the memory placement and timing.

The fix is commited in fwk122.
Comment 6 mikhail.voytenko 2009-10-05 10:34:28 UTC
*** Issue 105511 has been marked as a duplicate of this issue. ***
Comment 7 mikhail.voytenko 2009-10-05 10:49:12 UTC
*** Issue 105520 has been marked as a duplicate of this issue. ***
Comment 8 mikhail.voytenko 2009-10-05 11:07:08 UTC
Changing the Summary and OS accordingly since it is application and platform
independent problem, although it is much more difficult to reproduce it on
Widows than on Unix.
Comment 9 mikhail.voytenko 2009-10-06 10:32:22 UTC
mav->es: Please verify the issue.
Comment 10 eric.savary 2009-10-08 15:48:40 UTC
@MAV: as seen, fixed but failed.

As discussed, maybe 2 differents issues:
- I/O error when saving 2 shapes in binary format and reload
- loop when saving 2 shapes in binary format and reload

But both still happen in the CWS.

Please tell me if those are really 2 different tasks (i.e. needs a second issue)
Anyway, both should be fixed as showstopper.
Comment 11 eric.savary 2009-10-08 15:50:02 UTC
Reassigned back.
Comment 12 mikhail.voytenko 2009-10-09 13:27:48 UTC
Changing the Summary and OS back. The mentioned crash looks to be a different
problem and is covered by the issue 105520. The crash was fixed in the cws but
not the original scenario.
Comment 13 mikhail.voytenko 2009-11-16 19:56:47 UTC
The problem looks to be that the office produces a document that let any version
of the office hang ( at least all the versions I have tried ).

The problem is that the result .sda document contains header that contains own
size set to 0. As result the loop in
binfilter/bf_svx/source/svdraw/svx_svdpage.cxx:904
in method SdrObjList::Load()
never ends. The problem is that the SkipRecord() call on
binfilter/bf_svx/source/svdraw/svx_svdpage.cxx:1018
can not handle such a header correctly. It sets the current position to the
beginning of this header again and again.

mav->aw: cl has mentioned that you own currently the code that generates the
header. Could you please take a look.
Comment 14 mikhail.voytenko 2009-11-16 19:58:43 UTC
Created attachment 66143 [details]
The document that is generated in case of the scenario with three shapes ( that seems to happen only on some machines ).
Comment 15 mikhail.voytenko 2009-11-17 13:08:04 UTC
Just a remark.

That problem might be no regression at all. Since it is only reproducible on
some machines, it could be that it is a timing problem.

At least the binfilter project seems to contain no changes from OOo3.1 that
could cause the problem. One of possible reasons for the problem could be that a
new unknown entity is provided to the binfilter through flat-xml filter, and the
unexpected header is generated because of this. But that does not explain why it
is reproducible only on some machines.
Comment 16 Armin Le Grand 2009-11-17 16:36:06 UTC
AW: Could not reproduce on DEV300 m62, looking for other versions...
Comment 17 Armin Le Grand 2009-11-17 16:44:39 UTC
AW: Tried in all (eight) of my last local versions (CWSes), could not reproduce.
I'll try on linux now...
Comment 18 Armin Le Grand 2009-11-17 16:52:16 UTC
AW: Tried the same with around 6 pretty current linux versions, could also not
reproduce. I'll get a DEV300m59 now..
BTW: Who is saving in StarDraw5.0 format anymore? Is the 2nd used office a 5.2
version...? Those formats are pretty much deprecated nowadays...
Comment 19 Armin Le Grand 2009-11-17 17:07:26 UTC
AW: No more m59 available (as written in Version: field), oldest is m60. Getting
m60 and m64...
Checked both (linux), no problem found. I'll need a more specialized setup with
a machine where it happens reliably...
Comment 20 Armin Le Grand 2009-11-18 16:46:01 UTC
AW: Debugging on the virtual machine where it happens. Added all needed ebug
stuff and binfilter with needed parts with debug.
Experimented with a draw with 4 rectangles (count is relevant for the following
file offsets). When saving, the 2nd DrawingLayer page (regula, not Masterpage)
is started to be written at file position 3070 using a SdrIOHeader. When that
header is closed (at destruction, to debug, use  in
binfilter/bf_svx/source/svdraw/svx_svdpage.cxx), the correct BlockSize
(nBlkSize) of 537 is created and written to the file (see  in svx_svdio.cxx).
This is relevant since at read time, exactly at position 3070 the rerord is read
(see SdrModel::ReadData(...) in svx_svdmodel.cxx, break in line 1782 and wait
until nBufActualPos in rIn gets 3070). At read time, the wrong BlockSize
(nBlkSize) of null is read from the file.

Since i can clearly debug that the correct values get written but not read to
SvStream, i opt for an (evtl. system-dependent) error in SvStream (from tools
project) which is used by binfilter. When changes were made at the SvStream
class this would also explain why this happens in binfilter now, since tools is
one of the (rare) shared parts between office and binfilter.

3070 in hex is 0x0bfe, looking at the written file. Magic (4byte) and version
get read (2byte), then the nBlkSize (4byte). In the dumped file, there are no
zeros at that place, but in the buffer of SvStream.

AW->mav: Looks like problems with SvStream for me, have been changes made there
recently? I have debugged binfilter behaviour, all works as expected there.
Interestingly, 0x0bfe is odd and 2 and 4byte values are read from there, maybe
that causes problem nowadays?
Comment 21 mikhail.voytenko 2009-11-23 12:50:38 UTC
The problem is reproducible using the 4 shapes on all machines I have tried,
including a Solaris machine.

It looks to be dependent from the offset that the header has in the result
binary storage ( actually this offset is different from the offset used in the
header generation code ). The binary storage contains nulls at the offset in
this case.

The problem here is that the sal implementation writes sometimes directly to the
stream without update of the cache, although it is affected. That might allow to
generate corrupted files in some circumstances. The fix is to update the cache
accordingly.

mav->mh: I will attach the patch that allows to solve the problem. Could you
please review it.
Comment 22 mikhail.voytenko 2009-11-23 12:51:48 UTC
Created attachment 66285 [details]
The patch that solves the problem.
Comment 23 mikhail.voytenko 2009-11-23 12:53:13 UTC
mav->mhu: Could you please review the patch. The problem is that the buffer is
not always updated on writing.
Comment 24 matthias.huetsch 2009-11-23 13:29:38 UTC
mhu->mav: Well, I guess your patch would do, but have not completely verified.

I'll attach a much simpler patch, which makes semantics much more clear: 
invalidate the flushed buffer before attempting to write through to file.
Comment 25 matthias.huetsch 2009-11-23 13:31:01 UTC
Created attachment 66289 [details]
A simpler patch that solves the problem
Comment 26 mikhail.voytenko 2009-11-23 13:53:39 UTC
mav->mhu: Thank you for the patch.
Integrated in fwk129.
Comment 27 mikhail.voytenko 2009-11-24 10:31:58 UTC
mav->sba: Sending the issue to you since the original problem is reproducible on
your machine. Please verify the issue. Please also verify the scenario with 4
shapes, that seems to be good reproducible everywere.
Comment 28 michael.ruess 2009-11-30 13:08:51 UTC
*** Issue 107309 has been marked as a duplicate of this issue. ***
Comment 29 stefan.baltzer 2009-12-02 14:35:55 UTC
Verified in CWS fwk129.
Comment 30 stefan.baltzer 2010-01-12 13:17:12 UTC
OK in OOO320_m8. Closed.