framework/source/layoutmanager/toolbarlayoutmanager.cxx | 14 ++++++++------ framework/source/uielement/toolbarmanager.cxx | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-)
New commits: commit 84f2ff67a7e404febf710b1dc7f66d06745c503f Author: Michael Stahl <mst...@redhat.com> Date: Fri Dec 9 23:42:14 2016 +0100 framework: fix race in ToolBarManager creation ToolbarLayoutManager::createToolbar() may be called concurrently on different threads, and then it can happen that both threads want to create the same toolbar URL, see that it does not exist in line 457, then both release the SolarMutex and create a new ToolBarManager and the first inserts it and then the second overwrites it on line 514 without disposing the first one. The non-disposed extra ToolBarManager is kept alive because it is registered as a listener on the Frame. When the Frame::close() is called, the ToolbarLayoutManager is disposed, and that disposes all the ToolBarManagers it knows about, but not the extra one, which is then un-ref'd and then has a live VclPtr m_pToolBar, which asserts because the SolarMutex is not locked since commit e794ce1eef6730e5a46d5fb0aa6db2895ede85e7. (This commit is thanks to rr, which recorded the JunitTest_framework_complex execution and allowed debugging this.) Change-Id: I8f5333e8e36ac8ea347ef545e014ffc10501aebb diff --git a/framework/source/layoutmanager/toolbarlayoutmanager.cxx b/framework/source/layoutmanager/toolbarlayoutmanager.cxx index 4db5ca5..385fcd4 100644 --- a/framework/source/layoutmanager/toolbarlayoutmanager.cxx +++ b/framework/source/layoutmanager/toolbarlayoutmanager.cxx @@ -508,12 +508,14 @@ bool ToolbarLayoutManager::createToolbar( const OUString& rResourceURL ) UIElement& rElement = impl_findToolbar( rResourceURL ); if ( !rElement.m_aName.isEmpty() ) { - // Reuse a local entry so we are able to use the latest - // UI changes for this document. - implts_setElementData( rElement, xDockWindow ); - rElement.m_xUIElement = xUIElement; - bVisible = rElement.m_bVisible; - bFloating = rElement.m_bFloating; + // somebody else must have created it while we released + // the SolarMutex - just dispose our new instance and + // do nothing. (We have to dispose either the new or the + // existing m_xUIElement.) + aWriteLock.clear(); + uno::Reference<lang::XComponent> const xC(xUIElement, uno::UNO_QUERY); + xC->dispose(); + return false; } else { diff --git a/framework/source/uielement/toolbarmanager.cxx b/framework/source/uielement/toolbarmanager.cxx index 4c330b3..489ad87 100644 --- a/framework/source/uielement/toolbarmanager.cxx +++ b/framework/source/uielement/toolbarmanager.cxx @@ -200,7 +200,7 @@ ToolBarManager::ToolBarManager( const Reference< XComponentContext >& rxContext, ToolBarManager::~ToolBarManager() { assert(!m_aAsyncUpdateControllersTimer.IsActive()); - OSL_ASSERT( !m_pToolBar ); + assert(!m_pToolBar); // must be disposed by ToolbarLayoutManager OSL_ASSERT( !m_bAddedToTaskPaneList ); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits