summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2015-06-09 11:17:13 +0200
committerStephan Bergmann <sbergman@redhat.com>2015-06-09 11:18:21 +0200
commit0eb52534de536fbe33585c91f4f173653b184e24 (patch)
tree90596b8492272709bfe382416a4e1a915fb53884
parent2afe94dbfc85cbcde1399267379a466d527998a4 (diff)
Unlock SwCacheObj before potentially deleting it from SwCache
Running e.g. CppunitTest_sw_ooxmlexport under UBSan revealed problems with SwBorderAttrs instances with dangling pOwner pointers, which caused bad memory access during exit when ~SwCache calling ~SwBorderAttrs tried to use pOwner. The problem started with a4dee94afed9ade6ac50237c8d99a6e49d3bebc1 "tdf#91260: allow textboxes extending beyond the page bottom" (and hadn't changed with follow-up 2f779fc046c9afec04b4a4500b213e77aee51ae1 "tdf#91260: cleanup - textboxes extending beyond the page"): The call to pFrameFormat->SetFormatAttr(aSize); in SwAnchoredObjectPosition::_ImplAdjustVertRelPos ultimately calls SwCache::DeleteObj on the SwBorderAttrs instance that is locked by the SwBorderAttrAccess aAccess( SwFrm::GetCache(), this ); down the call stack in SwFlyFreeFrm::MakeAll. That means that SwCache::DeleteObj will return early without doing anything (apart from triggering the OSL_ENSURE "SwCache::Delete: object is locked."), leading to this leftover SwBorderAttrs instance causing trouble during ~SwCache. The scope of aAccess in SwFlyFreeFrm::MakeAll had always extended well past the uses of rAttrs (= *aAccess.Get()), covering also the if ( !mbValidPos ) block (that contains the call to MakeObjPos leading to the call of SwAnchoredObjectPosition::_ImplAdjustVertRelPos), ever since 84a3db80b4fd66c6854b3135b5f69b61fd828e62 "initial import." With cb19042f4395c97d123a27c6960d5e30d666c010 "New feature: vertical alignment for text frames: Layout part," an additional use of rAttrs (in MakeContentPos( rAttrs )) had been added after the block calling MakeObjPos. The hope is that (1) it is OK to release aAccess earlier, after any (original) uses of rAttrs, but before the call to MakeObjPos; and (2) it is OK to just set up a second aAccess/rAttrs for the later added use of rAttrs in the call to MakeContentPos. (That is, to punch a hole into the aAccess scope, so that ultimately SwCache::DeleteObj succeeds on a now-unlocked SwBorderAttrs.) Change-Id: I7cb9919b1c9d7c87464ac3a0fe1edfed5b46e122
-rw-r--r--sw/source/core/layout/flylay.cxx40
1 files changed, 22 insertions, 18 deletions
diff --git a/sw/source/core/layout/flylay.cxx b/sw/source/core/layout/flylay.cxx
index a5b27b6f5725..9b351c0c0f96 100644
--- a/sw/source/core/layout/flylay.cxx
+++ b/sw/source/core/layout/flylay.cxx
@@ -174,29 +174,33 @@ void SwFlyFreeFrm::MakeAll()
Format( &rAttrs );
bFormatHeightOnly = false;
}
+ }
- if ( !mbValidPos )
- {
- const Point aOldPos( (Frm().*fnRect->fnGetPos)() );
+ if ( !mbValidPos )
+ {
+ const Point aOldPos( (Frm().*fnRect->fnGetPos)() );
+ // #i26791# - use new method <MakeObjPos()>
+ // #i34753# - no positioning, if requested.
+ if ( IsNoMakePos() )
+ mbValidPos = true;
+ else
// #i26791# - use new method <MakeObjPos()>
- // #i34753# - no positioning, if requested.
- if ( IsNoMakePos() )
+ MakeObjPos();
+ if( aOldPos == (Frm().*fnRect->fnGetPos)() )
+ {
+ if( !mbValidPos && GetAnchorFrm()->IsInSct() &&
+ !GetAnchorFrm()->FindSctFrm()->IsValid() )
mbValidPos = true;
- else
- // #i26791# - use new method <MakeObjPos()>
- MakeObjPos();
- if( aOldPos == (Frm().*fnRect->fnGetPos)() )
- {
- if( !mbValidPos && GetAnchorFrm()->IsInSct() &&
- !GetAnchorFrm()->FindSctFrm()->IsValid() )
- mbValidPos = true;
- }
- else
- mbValidSize = false;
}
+ else
+ mbValidSize = false;
+ }
- if ( !m_bValidContentPos )
- MakeContentPos( rAttrs );
+ if ( !m_bValidContentPos )
+ {
+ SwBorderAttrAccess aAccess( SwFrm::GetCache(), this );
+ const SwBorderAttrs &rAttrs = *aAccess.Get();
+ MakeContentPos( rAttrs );
}
if ( mbValidPos && mbValidSize )