Issue 33728 - crash when using scripted list box in bug doc
Summary: crash when using scripted list box in bug doc
Status: CLOSED FIXED
Alias: None
Product: Base
Classification: Application
Component: code (show other issues)
Version: 680m51
Hardware: All All
: P2 Trivial (vote)
Target Milestone: OOo 2.0
Assignee: Frank Schönheit
QA Contact: issues@dba
URL:
Keywords: crash
Depends on:
Blocks:
 
Reported: 2004-09-02 08:53 UTC by Frank Schönheit
Modified: 2006-05-31 14:29 UTC (History)
2 users (show)

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


Attachments
document to reproduce the bug case (22.19 KB, application/vnd.sun.xml.calc)
2004-09-02 09:00 UTC, Frank Schönheit
no flags Details
proposed patch (11.41 KB, patch)
2004-09-02 09:46 UTC, Frank Schönheit
no flags Details | Diff
patch II (822 bytes, patch)
2004-09-02 11:33 UTC, Frank Schönheit
no flags Details | Diff
patch III (11.92 KB, patch)
2004-09-03 12:59 UTC, Frank Schönheit
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Frank Schönheit 2004-09-02 08:53:53 UTC
open the attched spreadsheet. It contains several sheets, and on every sheet a
listbox with the names of all sheets. Selecting an entry in the listbox
activates the respective sheet.

When doing this a few times (select a list box entry, see the sheet being
switched, select another entry, and so on), OOo finally crashes.
Comment 1 Frank Schönheit 2004-09-02 08:54:31 UTC
accepting
Comment 2 Frank Schönheit 2004-09-02 09:00:17 UTC
Created attachment 17501 [details]
document to reproduce the bug case
Comment 3 Frank Schönheit 2004-09-02 09:43:42 UTC
fs->ssa: This is yet another instance of a problem we already encountered a few
times.
Within VCL, WindowEvent listeners are called. Within this call, the VCL control
is destroyed for whatever reason. After returning from the call, the control
doesn't care or it being destroyed, but continues, for instance with accessing
some members ...

The solution is pretty easy, using the ImplDelData.

I am going to attach a patch, which fixes this, and some other potential areas
with the same problem.
Comment 4 Frank Schönheit 2004-09-02 09:46:04 UTC
Created attachment 17503 [details]
proposed patch
Comment 5 Frank Schönheit 2004-09-02 09:49:48 UTC
fs->ssa: The attached patch fixes the problem in a somewhat broader context.
The idea is that there are several controls which notfiy some listeners,
following the very same pattern:
  ImplCallEventListeners( <event_id> )
  maSomeHandler.Call( this )

I introduced a Control::ImplCallEventListenersAndHandler, which implements
exactly this pattern. Therein, an ImplDelData is used to ensure that that
nothing is notified/accessed in case the instance has been destroyed within the
first call.
Additionally, I modified all occurrences of this pattern in vcl/source/control,
which hopefully will spare us some more of this kind of bugs in the future ...

Could you please approve/deny this patch for checkin in VCL? Thanks :)
Comment 6 Frank Schönheit 2004-09-02 11:33:20 UTC
Created attachment 17504 [details]
patch II
Comment 7 Frank Schönheit 2004-09-02 11:35:04 UTC
The new patch fixes another crash, which still sporadically happens after the
first patch has been applied (but which is not caused by the first patch).

Still, there is one crash left: Now, when you often enough select an entry in
the list box, switching sheets this way, and close the document, this sometimes
(not always) crashs. Still investigating.
Comment 8 Frank Schönheit 2004-09-03 10:06:32 UTC
when using the two patches, I cannot provoke a crash on Linux, anymore. However,
Windows is still sporadically crashing.
Comment 9 stephan_schaefer 2004-09-03 11:16:47 UTC
ssa->fs: the first patch looks good, however, please remove the '_' in the
parameters of the new method ;)
the second patch is  missing an ImplRemoveDel() if it was not deleted.

Just guessing: the crash upon closing might be due to some handler still being
on the stack. So may be some action (like switching the sheets) has to be done
asynchronously. 
Comment 10 Frank Schönheit 2004-09-03 12:46:32 UTC
> the first patch looks good, however, please remove the '_' in the parameters
of the new
> method ;)

will do :)

> the second patch is  missing an ImplRemoveDel() if it was not deleted

oops. You're right, will correct this. Thanks for this round, I'll attach a new
patch.

> the crash upon closing might be due to some handler still being on the stack

Unfortunately not :(. It's some extremly strange crash upon destroying an
osl::Mutex, which I do not remotely understand :(

> So may be some action (like switching the sheets) has to be done asynchronously

I suppose this could solve the problem, but it wouldn't solve the root cause,
but only the symptoms - would it?
Comment 11 Frank Schönheit 2004-09-03 12:59:28 UTC
Created attachment 17533 [details]
patch III
Comment 12 Frank Schönheit 2004-09-03 13:00:13 UTC
"patch III" merges the new versions of the previous two.
Comment 13 stephan_schaefer 2004-09-03 17:32:17 UTC
patch looks good now - of course....

doing things asynchronously is sometimes the only way, but in your case you may
be right about the symptoms. didnÄt know about the mutex when I started guessing.
Comment 14 Frank Schönheit 2004-09-07 16:25:03 UTC
fs->fs:
interesting finding:
When removing the script events from the buttons contained in the document
(context menu -> "Control ..." -> "Events" -> "when initiated"), then the
crash-upon-closing-the-document does not happen. Strange.
Comment 15 Frank Schönheit 2004-09-09 13:32:21 UTC
After struggling with this for more than a week, I made the notification
asynchronous ... There were plenty of places (not only the ones addressed in the
patch III) which did not survive synchronous notifications (because objects on
the stack were killed within the notification). Fixing them all would have been
too much hazzle ....

fixed in CWS dba17
Comment 16 Frank Schönheit 2004-11-26 08:42:59 UTC
looks good in m63 -> closing