Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[x86] CTCatalog - Font Functions - crashes when attempting to change Select nameKey #1865

Open
ashvarma opened this issue Jan 27, 2017 · 5 comments

Comments

@ashvarma
Copy link
Contributor

This is on the 1701 payload that's in master - 20170127.1

@aballway
Copy link
Contributor

Seems to be introduced in 88975ca

@jaredhms
Copy link
Contributor

jaredhms commented Jan 30, 2017

We recently shifted Islandwood over to not using queued CATransactions for its UIElement property and hierarchy management; instead we perform the property/hierarchy changes synchronously on the backing Xaml elements as they’re issued by the app. This change was required to fix several other bugs, and it should also lead to a much more simplified Islandwood implementation layer. It’s worked pretty well so far, but it’s uncovered an interesting issue with ScrollViewer’s ScrollViewerViewChangedEvent.

CTCatalog has a UIPickerView within a UITableView. Selecting an item in the UIPickerView results in reloading the entire parent UITableView’s data, which ends up removing and re-adding all of that UITableView’s contents. We don’t actually delete/recreate the UIPickerView, but it is temporarily removed from the UIElement tree, and then added back as the UITableView rebuilds its content. This may sound odd, but it’s a perfectly valid use of UITableView’s reloadData method.

Just as on iOS, our UIPickerView is built upon our UIScrollView (which is backed by a Xaml ScrollViewer). The UIPickerView’s ‘selection changed’ event is fired as a result of the UIScrollView’s ‘scrollViewDidScroll’ event, which is keyed off of the Xaml ScrollViewer’s ScrollViewerViewChangedEvent. This is where the issue lies. The ScrollViewerViewChangedEvent is fired during Arrange, and removing the ScrollViewer from the UI during this event handler leads to an AV in CFrameworkElement::ArrangeCore because the ScrollViewer’s m_pLayoutStorage was deleted.

Here's the full stack where we remove the ScrollViewer from the UIElement tree.

   Windows.UI.Xaml.dll!CUIElement::Shutdown() Line 7434 C++
  Windows.UI.Xaml.dll!CUIElementCollection::OnRemoveFromCollection(CDependencyObject * pDO, int iPreviousIndex) Line 207     C++
  Windows.UI.Xaml.dll!CDOCollection::RemoveAt(unsigned int nIndex) Line 749      C++
  Windows.UI.Xaml.dll!CUIElementCollection::Remove(unsigned int nIndex, CDependencyObject * pObject) Line 1291  C++
  Windows.UI.Xaml.dll!CUIElementCollection::RemoveAt(unsigned int nIndex) Line 1344    C++
  Windows.UI.Xaml.dll!Collection_RemoveAt(CCollection * pCollection, unsigned int nIndex) Line 2196    C++

Windows.UI.Xaml.dll!DirectUI::PresentationFrameworkCollectionTemplateBase<Windows::UI::Xaml::UIElement >::RemoveAt(unsigned int index) Line 872 C++
UIKIT.DLL!LayerProxy::RemoveFromSuperLayer() Line 356 C++
UIKIT.DLL!LayerTransaction::_QueueMovement(char16_t * layerMovement, unsigned int) Line 240 Objective-C++
UIKIT.DLL!LayerTransaction::RemoveLayer(const std::shared_ptr & layer) Line 192 Objective-C++
QUARTZCORE.DLL!+[CATransaction _removeLayer:](objc_class * self, objc_selector * _cmd, CALayer * layer) Line 240 Objective-C++
QUARTZCORE.DLL!-[CALayer removeFromSuperlayer](CALayer * self, objc_selector * _cmd) Line 844 Objective-C++
UIKIT.DLL!-[UIView removeFromSuperview](UIView * self, objc_selector * _cmd) Line 1630 Objective-C++
UIKIT.DLL!-[UITableView reloadData](UITableView * self, objc_selector * _cmd) Line 1255 Objective-C++
CTCatalog.exe!-[CTCFontTestViewController refreshTests](CTCFontTestViewController * const self, objc_selector * _cmd) Line 330 Objective-C++
CTCatalog.exe!-[CTCFontTestViewController pickerView:didSelectRow:inComponent:](CTCFontTestViewController * const self, objc_selector * _cmd, id * row, int component, int) Line 362 Objective-C++
UIKIT.DLL!-[UIPickerView _subCellSelected:fromPicker:](UIPickerView * self, objc_selector * _cmd, int row, UIPickerSubView * fromPicker) Line 633 Objective-C++
UIKIT.DLL!-[UIPickerSubView scrollViewDidScroll:](UIPickerSubView * self, objc_selector * _cmd, objc_object * scrollView) Line 303 Objective-C++
UIKIT.DLL!-[UIScrollView _refreshOrigin](UIScrollView * const self, objc_selector * _cmd) Line 897 Objective-C++
UIKIT.DLL!changeContentOffset(std::memory_order offset) Line 928 Objective-C++
UIKIT.DLL!40-[UIScrollView setupViewChangedHandler]block_invoke(void * .block_descriptor, RTObject *) Line 466 Objective-C++
OBJCUWP_WINDOWS_UI_XAML.DLL!EventHandler_Windows_UI_Xaml_Controls_ScrollViewerViewChangedEventArgs::Invoke(IInspectable * arg0, Controls::IScrollViewerViewChangedEventArgs * arg1) Line 295 Objective-C++
Windows.UI.Xaml.dll!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,Windows::Foundation::IEventHandler<Windows::UI::Xaml::Controls::ScrollViewerViewChangedEventArgs *>,IInspectable,Windows::UI::Xaml::Controls::IScrollViewerViewChangedEventArgs>::Raise(IInspectable * pSource, Windows::UI::Xaml::Controls::IScrollViewerViewChangedEventArgs * pArgs) Line 273 C++
Windows.UI.Xaml.dll!DirectUI::ScrollViewer::RaiseViewChanged(unsigned char isIntermediate) Line 14033 C++
Windows.UI.Xaml.dll!DirectUI::ScrollViewer::FlushViewChanged(HRESULT hr) Line 14127 C++
Windows.UI.Xaml.dll!DirectUI::ScrollViewer::InvalidateScrollInfoImpl() Line 4558 C++
Windows.UI.Xaml.dll!DirectUI::ScrollContentPresenter::VerifyScrollData(Windows::Foundation::Size viewport, Windows::Foundation::Size extent) Line 2898 C++
Windows.UI.Xaml.dll!DirectUI::ScrollContentPresenter::ArrangeOverride(Windows::Foundation::Size finalSize, Windows::Foundation::Size * pReturnValue) Line 1978 C++
Windows.UI.Xaml.dll!DirectUI::FrameworkElementGenerated::ArrangeOverrideProtected(Windows::Foundation::Size finalSize, Windows::Foundation::Size * pReturnValue) Line 704 C++
Windows.UI.Xaml.dll!DirectUI::FrameworkElement::ArrangeOverrideFromCore(CFrameworkElement * nativeTarget, float inWidth, float inHeight, float * outWidth, float * outHeight) Line 365 C++
Windows.UI.Xaml.dll!CFrameworkElement::ArrangeCore(XRECTF_WH finalRect) Line 1797 C++
Windows.UI.Xaml.dll!CUIElement::ArrangeInternal(XRECTF_WH finalRect) Line 4290 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4105 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CUIElement::Arrange(XRECTF_WH finalRect) Line 4181 C++
Windows.UI.Xaml.dll!CLayoutManager::UpdateLayout(unsigned int controlWidth, unsigned int controlHeight) Line 268 C++
Windows.UI.Xaml.dll!CCoreServices::NWDrawTree(HWWalk * pHWWalk, CWindowRenderTarget * pRenderTarget, VisualTree * pVisualTree, unsigned int forceRedraw, XRECT_WH * prcDirtyRect) Line 7094 C++
Windows.UI.Xaml.dll!CCoreServices::NWDrawMainTree(CWindowRenderTarget * pIRenderTarget, bool fForceRedraw, XRECT_WH * prcDirtyRect) Line 6887 C++
Windows.UI.Xaml.dll!CWindowRenderTarget::Draw(CCoreServices * pCore, unsigned int fForceRedraw, XRECT_WH * prcDirtyRect) Line 138 C++
Windows.UI.Xaml.dll!CXcpBrowserHost::OnTick() Line 528 C++
Windows.UI.Xaml.dll!CXcpDispatcher::Tick() Line 1349 C++
Windows.UI.Xaml.dll!CXcpBrowserHost::OnPaint() Line 1654 C++
Windows.UI.Xaml.dll!CXcpDispatcher::OnPaint() Line 571 C++
Windows.UI.Xaml.dll!CControlBase::OnPaint() Line 212 C++
Windows.UI.Xaml.dll!CJupiterControl::Paint() Line 527 C++
Windows.UI.Xaml.dll!CJupiterControl::HandleWindowMessage(unsigned int uMsg, unsigned int wParam, long lParam) Line 282 C++
Windows.UI.Xaml.dll!CJupiterWindow::CoreWindowSubclassProc(HWND
* hwnd, unsigned int uMsg, unsigned int wParam, long lParam) Line 1072 C++
Windows.UI.Xaml.dll!CJupiterWindow::StaticCoreWindowSubclassProc(HWND
* hwnd, unsigned int uMsg, unsigned int wParam, long lParam) Line 519 C++
user32.dll!_InternalCallWinProc@20() Line 116 Unknown
user32.dll!UserCallWinProcCheckWow(ACTIVATION_CONTEXT * pActCtx, void * pfn, HWND
* hwnd, _WM_VALUE msg, unsigned int wParam, long lParam, void * fEnableLiteHooks, int) Line 233 C
user32.dll!DispatchClientMessage(tagWND * pwnd, _WM_VALUE message, unsigned int wParam, long lParam, unsigned long pfn) Line 3462 C
user32.dll!__fnDWORD(_FNDWORDMSG * pmsg) Line 1621 C
ntdll.dll!_KiUserCallbackDispatcher@12() Line 517 Unknown
user32.dll!DispatchMessageW(const tagMSG * lpMsg) Line 1039 C
Windows.UI.dll!Windows::UI::Core::CDispatcher::ProcessMessage(int bDrainQueue, int * pbWindowMessagesProcessed, int * pbInvokeItemProcessed) Line 316 C++
Windows.UI.dll!Windows::UI::Core::CDispatcher::WaitAndProcessMessagesInternal(int bRunAlwaysOnce, void * hEventWait) Line 1768 C++
Windows.UI.dll!Windows::UI::Core::CDispatcher::ProcessEvents(Windows::UI::Core::CoreProcessEventsOption options) Line 570 C++
Windows.UI.Xaml.dll!CJupiterWindow::RunCoreWindowMessageLoop() Line 1221 C++
Windows.UI.Xaml.dll!CJupiterControl::RunMessageLoop() Line 903 C++
Windows.UI.Xaml.dll!DirectUI::DXamlCore::RunMessageLoop() Line 2039 C++
Windows.UI.Xaml.dll!DirectUI::FrameworkView::Run() Line 101 C++
twinapi.appcore.dll!Windows::ApplicationModel::Core::CoreApplicationViewAgileContainer::RuntimeClassInitialize::__l8::(void * pv) Line 1130 C++
SHCore.dll!_WrapperThreadProc(void * pv) Line 321 C++
kernel32.dll!BaseThreadInitThunk(unsigned long RunProcessInit, long(__stdcall
)(void ) StartAddress, void * Argument) Line 64 C
ntdll.dll!__RtlUserThreadStart(long(__stdcall
)(void ) StartAddress, void * Argument) Line 997 C
ntdll.dll!_RtlUserThreadStart(long(__stdcall
)(void *) StartAddress, void * Argument) Line 914 C

…which results in an AV at Windows.UI.Xaml.dll!CFrameworkElement::ArrangeCore(XRECTF_WH finalRect) Line 1855:

   RenderSize = innerInkSize;

   ...

   #define RenderSize GetLayoutStorage()->m_size

It seems a bit odd that ScrollViewer raises ScrollViewerViewChangedEvent within Arrange; MSDN doesn’t call this out as a limitation. Ideally we’d be able to do whatever we want within the ScrollViewerViewChangedEvent, including removing the ScrollViewer from the UIElement tree.

@jaredhms
Copy link
Contributor

We have a few options;

  1. Update the app to opt into useLegacyBatchedCATransactions
  2. Update the app to dispatch_async its call to reload the UITableView data on a subsequent run of the UI thread
  3. Update UIScrollViewer to dispatch_async its call to scrollViewDidScroll:

We'll need to discuss our options here; I think 3 sounds the most promising if we can prove that it won't cause other issues.

jaredhms added a commit to jaredhms/WinObjC that referenced this issue Jan 31, 2017
…Xaml ScrollViewer's ScrollViewerViewChanged event leads to an AV in Xaml if the delegate removes the UIScrollView from the Xaml UIElement tree. To work around the issue, push the delegate call to a subsequent run of the UI thread.

Fixes microsoft#1865.
@ArnOmsft ArnOmsft changed the title [x86] CTCatalog - Font Fucntions - crashes when attempting to change Select nameKey [x86] CTCatalog - Font Functions - crashes when attempting to change Select nameKey Jan 31, 2017
jaredhms added a commit to jaredhms/WinObjC that referenced this issue Feb 6, 2017
…Xaml ScrollViewer's ScrollViewerViewChanged event leads to an AV in Xaml if the delegate removes the UIScrollView from the Xaml UIElement tree. To work around the issue, push the delegate call to a subsequent run of the UI thread.

Fixes microsoft#1865.
@jaredhms jaredhms assigned tadam-msft and unassigned jaredhms Mar 27, 2017
@jaredhms
Copy link
Contributor

@tadam-msft, what do you want to do with this one?

@tadam-msft
Copy link
Member

Option 3

@tadam-msft tadam-msft assigned jaredhms and unassigned tadam-msft Apr 3, 2017
@jaredhms jaredhms assigned rajsesh and unassigned jaredhms May 11, 2017
@yiyang-msft yiyang-msft assigned yiyang-msft and unassigned rajsesh May 11, 2017
@rajsesh rajsesh added this to the Backlog milestone Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants