Issue 111452 - framework: assert in TransactionManager when closing window with quickstart enabled
Summary: framework: assert in TransactionManager when closing window with quickstart e...
Status: CLOSED FIXED
Alias: None
Product: General
Classification: Code
Component: code (show other issues)
Version: DEV300m77
Hardware: All Linux, all
: P3 Trivial (vote)
Target Milestone: OOo 3.3
Assignee: carsten.driesner
QA Contact: issues@framework
URL:
Keywords:
: 110623 (view as issue list)
Depends on:
Blocks:
 
Reported: 2010-05-07 12:19 UTC by dtardon
Modified: 2010-08-03 07:58 UTC (History)
2 users (show)

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


Attachments
possible fix (1.20 KB, patch)
2010-05-07 12:20 UTC, dtardon
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description dtardon 2010-05-07 12:19:44 UTC
More specifically, I'm getting the following two asserts:

Error: ASSERT:
	TransactionManager...
	"Owner instance already closed. Call was rejected!"
 From File
/home/dtardon/work/upstream/openoffice.org/build/devel/framework/source/threadhelp/transactionmanager.cxx
at Line 396
Error: ASSERT:
	CloseDispatcher::impl_asyncCallback
	"Congratulation! You found the reason for bug #120310#. Please contact the
right developer and show him a scenario, which trigger this bug. THX."
 From File
/home/dtardon/work/upstream/openoffice.org/build/devel/framework/source/dispatch/closedispatcher.cxx
at Line 454


How to reproduce:

1. start oo.o and ensure that you don't have opened anything but start center
2. enable quickstarter
3. open one writer window
4. close the window
Comment 1 dtardon 2010-05-07 12:20:48 UTC
Created attachment 69358 [details]
possible fix
Comment 2 Mathias_Bauer 2010-05-11 13:47:27 UTC
Thanks for the patch, it helped me to find the culprit. OTOH I solved the
problem differently. It took some time to analyze this problem.

The reason for the assertion is that getController() is called at a disposed
frame and its TransactionGuard complains about that by showing the assertion
message. The offending call happens in the AsyncCallback of the CloseDispatcher
(the code that you have patched).

The idea of this code is that once a Controller has been suspended in
preparation for closing the frame, this suspension must be removed when the
closing of the frame failed. Otherwise the UI inside the frame would be
dysfunctional and perhaps a crash is not far away.

The code makes a mistake by assuming that "bSuccess" being sal_False is enough
to detect the need to "unsuspend", but unfortunately the variable is can have
this value also if the frame was closed successfully, but the Desktop could not
be terminated. This is the case e.g. if a Quickstarter is active.

Your patch just checks if the frame was actually destroyed, but it will fail in
case any other code still has a "hard" reference to the frame.

My first attempt was to have two variables "bSuccess" (for the overall access of
the whole method) and "bCloseSuccess" that will be true only if the frame was
closed successfully by the CloseDispatcher. Unfortunately after some thinking I
discovered scenarios where the Controller was suspended, but the frame was
closed or tried to be closed elsewhere, so "bCloseSuccess" is not able to
provide the needed information.

What we want to know is: was the frame disposed so that calling getController()
is not necessary? A possible solution would be to register an EventListener at
the frame that notifies the CloseDispatcher about the disposing event. But IMHO
that would be overkill. I think it is OK to remove the TransactionGuard from the
Frame::getController() method. This makes clear that calling getController() at
a disposed frame is not a mistake. I can't think of a situation where this would
cause trouble: getController() just returns a copy of a member variable that is
not guaranteed to be not empty.
Comment 3 Mathias_Bauer 2010-05-18 16:55:08 UTC
target 3.3
Comment 4 Mathias_Bauer 2010-06-04 12:27:08 UTC
please verify
Comment 5 carsten.driesner 2010-06-04 13:20:39 UTC
cd: Verified with non-pro build.
Comment 6 400guy 2010-06-27 20:20:48 UTC
*** Issue 110623 has been marked as a duplicate of this issue. ***
Comment 7 dtardon 2010-08-03 07:58:13 UTC
integrated in DEV300_m84