Ticket #4209 - Crash of FreeCAD 0.19 on undo

[TheMarkster]

Here is output from a release build with the logging code added:

VPS::rebuildConstraintsVisual, constraint 57, geo1 is null, First: 35, geomsize is: 34, extgeo is: 2

[TheMarkster]

Call stack from a debug build

Code: Select all

SketcherGui_d.pyd!SketcherGui::ViewProviderSketch::rebuildConstraintsVisual() Line 5449
	at e:\free-cad-code\src\mod\sketcher\gui\viewprovidersketch.cpp(5449)
SketcherGui_d.pyd!SketcherGui::ViewProviderSketch::draw(bool temp, bool rebuildinformationlayer) Line 4307
	at e:\free-cad-code\src\mod\sketcher\gui\viewprovidersketch.cpp(4307)
SketcherGui_d.pyd!SketcherGui::ViewProviderSketch::updateData(const App::Property * prop) Line 5576
	at e:\free-cad-code\src\mod\sketcher\gui\viewprovidersketch.cpp(5576)
FreeCADGui_d.dll!Gui::ViewProvider::update(const App::Property * prop) Line 300
	at e:\free-cad-code\src\gui\viewprovider.cpp(300)
FreeCADGui_d.dll!Gui::ViewProviderDocumentObject::update(const App::Property * prop) Line 327
	at e:\free-cad-code\src\gui\viewproviderdocumentobject.cpp(327)
FreeCADGui_d.dll!Gui::Document::slotChangedObject(const App::DocumentObject & Obj, const App::Property & Prop) Line 804
	at e:\free-cad-code\src\gui\document.cpp(804)
FreeCADGui_d.dll!boost::_mfi::mf2<void,Gui::Document,App::DocumentObject const &,App::Property const &>::operator()(Gui::Document * p, const App::DocumentObject & a1, const App::Property & a2) Line 281
	at e:\freecadlibs\include\boost\bind\mem_fn_template.hpp(281)
FreeCADGui_d.dll!boost::_bi::list3<boost::_bi::value<Gui::Document *>,boost::arg<1>,boost::arg<2> >::operator()<boost::_mfi::mf2<void,Gui::Document,App::DocumentObject const &,App::Property const &>,boost::_bi::rrlist2<App::DocumentObject const &,App::Property const &> >(boost::_bi::type<void> __formal, boost::_mfi::mf2<void,Gui::Document,App::DocumentObject const &,App::Property const &> & f, boost::_bi::rrlist2<App::DocumentObject const &,App::Property const &> & a, int __formal) Line 399
	at e:\freecadlibs\include\boost\bind\bind.hpp(399)
FreeCADGui_d.dll!boost::_bi::bind_t<void,boost::_mfi::mf2<void,Gui::Document,App::DocumentObject const &,App::Property const &>,boost::_bi::list3<boost::_bi::value<Gui::Document *>,boost::arg<1>,boost::arg<2> > >::operator()<App::DocumentObject const &,App::Property const &>(const App::DocumentObject & a1, const App::Property & a2) Line 1319
	at e:\freecadlibs\include\boost\bind\bind.hpp(1319)
FreeCADGui_d.dll!boost::detail::function::void_function_obj_invoker2<boost::_bi::bind_t<void,boost::_mfi::mf2<void,Gui::Document,App::DocumentObject const &,App::Property const &>,boost::_bi::list3<boost::_bi::value<Gui::Document *>,boost::arg<1>,boost::arg<2> > >,void,App::DocumentObject const &,App::Property const &>::invoke(boost::detail::function::function_buffer & function_obj_ptr, const App::DocumentObject & a0, const App::Property & a1) Line 160
	at e:\freecadlibs\include\boost\function\function_template.hpp(160)
FreeCADApp_d.dll!boost::function2<void,App::DocumentObject const &,App::Property const &>::operator()(const App::DocumentObject & a0, const App::Property & a1) Line 770
	at e:\freecadlibs\include\boost\function\function_template.hpp(770)
FreeCADApp_d.dll!boost::signals2::detail::call_with_tuple_args<boost::signals2::detail::void_type>::m_invoke<boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)>,0,1,App::DocumentObject const &,App::Property const &>(boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> & func, boost::signals2::detail::unsigned_meta_array<0,1> __formal, const std::tuple<App::DocumentObject const &,App::Property const &> & args, void * __formal) Line 106
	at e:\freecadlibs\include\boost\signals2\detail\variadic_slot_invoker.hpp(106)
FreeCADApp_d.dll!boost::signals2::detail::call_with_tuple_args<boost::signals2::detail::void_type>::operator()<boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)>,App::DocumentObject const &,App::Property const &,2>(boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> & func, const std::tuple<App::DocumentObject const &,App::Property const &> & args, boost::mpl::size_t<2> __formal) Line 90
	at e:\freecadlibs\include\boost\signals2\detail\variadic_slot_invoker.hpp(90)
FreeCADApp_d.dll!boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>::operator()<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > >(const boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > & connectionBody) Line 133
	at e:\freecadlibs\include\boost\signals2\detail\variadic_slot_invoker.hpp(133)
FreeCADApp_d.dll!boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> >::dereference() Line 110
	at e:\freecadlibs\include\boost\signals2\detail\slot_call_iterator.hpp(110)
FreeCADApp_d.dll!boost::iterators::iterator_core_access::dereference<boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > >(const boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > & f) Line 551
	at e:\freecadlibs\include\boost\iterator\iterator_facade.hpp(551)
FreeCADApp_d.dll!boost::iterators::detail::iterator_facade_base<boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> >,boost::signals2::detail::void_type,boost::iterators::single_pass_traversal_tag,boost::signals2::detail::void_type const &,__int64,0,0>::operator*() Line 657
	at e:\freecadlibs\include\boost\iterator\iterator_facade.hpp(657)
FreeCADApp_d.dll!boost::signals2::optional_last_value<void>::operator()<boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > >(boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > first, boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > last) Line 57
	at e:\freecadlibs\include\boost\signals2\optional_last_value.hpp(57)
FreeCADApp_d.dll!boost::signals2::detail::combiner_invoker<void>::operator()<boost::signals2::optional_last_value<void>,boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > >(boost::signals2::optional_last_value<void> & combiner, boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > first, boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::variadic_slot_invoker<boost::signals2::detail::void_type,App::DocumentObject const &,App::Property const &>,std::_List_iterator<std::_List_val<std::_List_simple_types<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > > > >,boost::signals2::detail::connection_body<std::pair<enum boost::signals2::detail::slot_meta_group,boost::optional<int> >,boost::signals2::slot<void __cdecl(App::DocumentObject const &,App::Property const &),boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)> >,boost::signals2::mutex> > last) Line 64
	at e:\freecadlibs\include\boost\signals2\detail\result_type_wrapper.hpp(64)
FreeCADApp_d.dll!boost::signals2::detail::signal_impl<void __cdecl(App::DocumentObject const &,App::Property const &),boost::signals2::optional_last_value<void>,int,std::less<int>,boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)>,boost::function<void __cdecl(boost::signals2::connection const &,App::DocumentObject const &,App::Property const &)>,boost::signals2::mutex>::operator()(const App::DocumentObject & <args_0>, const App::Property & <args_1>) Line 242
	at e:\freecadlibs\include\boost\signals2\detail\signal_template.hpp(242)
FreeCADApp_d.dll!boost::signals2::signal<void __cdecl(App::DocumentObject const &,App::Property const &),boost::signals2::optional_last_value<void>,int,std::less<int>,boost::function<void __cdecl(App::DocumentObject const &,App::Property const &)>,boost::function<void __cdecl(boost::signals2::connection const &,App::DocumentObject const &,App::Property const &)>,boost::signals2::mutex>::operator()(const App::DocumentObject & <args_0>, const App::Property & <args_1>) Line 723
	at e:\freecadlibs\include\boost\signals2\detail\signal_template.hpp(723)
FreeCADApp_d.dll!App::Document::onChangedProperty(const App::DocumentObject * Who, const App::Property * What) Line 1487
	at e:\free-cad-code\src\app\document.cpp(1487)
FreeCADApp_d.dll!App::DocumentObject::onChanged(const App::Property * prop) Line 743
	at e:\free-cad-code\src\app\documentobject.cpp(743)
Part_d.pyd!Part::Feature::onChanged(const App::Property * prop) Line 563
	at e:\free-cad-code\src\mod\part\app\partfeature.cpp(563)
Sketcher_d.pyd!Sketcher::SketchObject::onChanged(const App::Property * prop) Line 6832
	at e:\free-cad-code\src\mod\sketcher\app\sketchobject.cpp(6832)
FreeCADApp_d.dll!App::Property::hasSetValue() Line 217
	at e:\free-cad-code\src\app\property.cpp(217)
Sketcher_d.pyd!Sketcher::PropertyConstraintList::setValues(const std::vector<Sketcher::Constraint *,std::allocator<Sketcher::Constraint *> > & lValue) Line 188
	at e:\free-cad-code\src\mod\sketcher\app\propertyconstraintlist.cpp(188)
Sketcher_d.pyd!Sketcher::PropertyConstraintList::Paste(const App::Property & from) Line 360
	at e:\free-cad-code\src\mod\sketcher\app\propertyconstraintlist.cpp(360)
FreeCADApp_d.dll!App::TransactionObject::applyChn(App::Document & __formal, App::TransactionalObject * pcObj, bool __formal) Line 352
	at e:\free-cad-code\src\app\transactions.cpp(352)
FreeCADApp_d.dll!App::Transaction::apply(App::Document & Doc, bool forward) Line 177
	at e:\free-cad-code\src\app\transactions.cpp(177)
FreeCADApp_d.dll!App::Document::undo(int id) Line 971
	at e:\free-cad-code\src\app\document.cpp(971)
FreeCADGui_d.dll!Gui::Document::undo(int iSteps) Line 2211
	at e:\free-cad-code\src\gui\document.cpp(2211)
FreeCADGui_d.dll!Gui::View3DInventor::onMsg(const char * pMsg, const char * * ppReturn) Line 654
	at e:\free-cad-code\src\gui\view3dinventor.cpp(654)
FreeCADGui_d.dll!Gui::Application::sendMsgToActiveView(const char * pMsg, const char * * ppReturn) Line 899
	at e:\free-cad-code\src\gui\application.cpp(899)
FreeCADGui_d.dll!StdCmdUndo::activated(int iMsg) Line 816
	at e:\free-cad-code\src\gui\commanddoc.cpp(816)
FreeCADGui_d.dll!Gui::Command::invoke(int i, Gui::Command::TriggerSource trigger) Line 409
	at e:\free-cad-code\src\gui\command.cpp(409)
FreeCADGui_d.dll!Gui::Action::onActivated() Line 97
	at e:\free-cad-code\src\gui\action.cpp(97)
FreeCADGui_d.dll!Gui::Action::qt_static_metacall(QObject * _o, QMetaObject::Call _c, int _id, void * * _a) Line 74
	at e:\free-cad-bin\src\gui\moc_action.cpp(74)
Qt5Cored.dll!00007ffae8755686()
Qt5Cored.dll!00007ffae8754dc8()
Qt5Widgetsd.dll!00007ffaef90e75d()
Qt5Widgetsd.dll!00007ffaef90dac2()
Qt5Widgetsd.dll!00007ffaef92fb15()
Qt5Widgetsd.dll!00007ffaefcaae34()
Qt5Widgetsd.dll!00007ffaefaff48d()
Qt5Widgetsd.dll!00007ffaefafeb27()
Qt5Widgetsd.dll!00007ffaefcaa94c()
Qt5Widgetsd.dll!00007ffaef99bb4d()
Qt5Widgetsd.dll!00007ffaefafe5bb()
Qt5Widgetsd.dll!00007ffaefcaa7a0()
Qt5Widgetsd.dll!00007ffaef93c0be()
Qt5Widgetsd.dll!00007ffaef937368()
FreeCADGui_d.dll!Gui::GUIApplication::notify(QObject * receiver, QEvent * event) Line 92
	at e:\free-cad-code\src\gui\guiapplication.cpp(92)
Qt5Cored.dll!00007ffae86fe236()
Qt5Cored.dll!00007ffae86fe0db()
Qt5Widgetsd.dll!00007ffaef93e7c4()
Qt5Widgetsd.dll!00007ffaef9e6c33()
Qt5Widgetsd.dll!00007ffaef9e5276()
Qt5Widgetsd.dll!00007ffaef93c0be()
Qt5Widgetsd.dll!00007ffaef936b5f()
FreeCADGui_d.dll!Gui::GUIApplication::notify(QObject * receiver, QEvent * event) Line 92
	at e:\free-cad-code\src\gui\guiapplication.cpp(92)
Qt5Cored.dll!00007ffae86fe236()
Qt5Cored.dll!00007ffae86fe0db()
Qt5Guid.dll!00007ffae9101144()
Qt5Guid.dll!00007ffae9103ec3()
Qt5Guid.dll!00007ffae90d22e5()
qwindowsd.dll!00007ffaf5b64942()
Qt5Cored.dll!00007ffae87bef98()
user32.dll!00007ffb394c5c0d()
user32.dll!00007ffb394c5602()
Qt5Cored.dll!00007ffae87bf888()
qwindowsd.dll!00007ffaf5b648f4()
Qt5Cored.dll!00007ffae86f8463()
Qt5Cored.dll!00007ffae86f869e()
Qt5Cored.dll!00007ffae86fbd2f()
Qt5Guid.dll!00007ffae90fe4c8()
Qt5Widgetsd.dll!00007ffaef93650a()
FreeCADGui_d.dll!Gui::Application::runApplication() Line 2148
	at e:\free-cad-code\src\gui\application.cpp(2148)
FreeCAD_d.exe!main(int argc, char * * argv) Line 302
	at e:\free-cad-code\src\main\maingui.cpp(302)
FreeCAD_d.exe!WinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, char * __formal, int __formal) Line 104
	at c:\users\qt\work\qt\qtbase\src\winmain\qtmain_win.cpp(104)
FreeCAD_d.exe!invoke_main() Line 107
	at d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl(107)
FreeCAD_d.exe!__scrt_common_main_seh() Line 288
	at d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl(288)
FreeCAD_d.exe!__scrt_common_main() Line 331
	at d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl(331)
FreeCAD_d.exe!WinMainCRTStartup() Line 17
	at d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_winmain.cpp(17)
kernel32.dll!00007ffb386e7bd4()
ntdll.dll!00007ffb39dace51()

[abdullah]

I wanted to say that I am working on this ticket.

I have started a solution, but I am having some busy days at work. I hope to finish for the weekend.

[abdullah]

Reproducibility:

The crash can be systematically be generated by deleting this geometry and constraint:

Screenshot_20200610_184038.png (10.63 KiB) Viewed 894 times

Maybe we can think of a controlling instance that observes the two properties when a transaction is performed and notifies the sketch viewprovider when the transaction is finished.

I have taken a look into the issue.

Indeed the problem originates from the lack of sync of sketchobject properties during the undo, which are used to update the viewprovider. However, there is a long standing further issue, which relates to the undo function not reconstructing the invalid geometry indices of PropertyConstraintList. Two recomputes are necessary for this (when none should be needed). I explain it below.

I have a working branch here, to be used for the discussion of the code. In particular this commit.Yes, the crash is solved, but I think we could do better.

The main issue

The crash is generated because the ViewProvider is updated (via updateData) on every property change. This leads to inconsistent Geometry and Constraints.

The flow goes like this:
=> App:Document::undo => applchn is called on every transactional object (e.g. properties) affected => each causes Paste on the property =>each causes => SketchObject::OnChanged and App::DocumentObject::OnChanged => the latter calling onChangeProperty (for each property in the transaction) => Gui::Document::slotChangedObject => ViewProviderSketch::updateData (one update for each property).

This causes that the ViewProvider is updated when Constraints and Geometry indexes are not matching. Because the geometry indices stored in the Constraints are not recalculated on pasting the property (and it is not a good idea to do it, as the geometry paste may come next). In fact there could be more issues like wrong vertices index in SketchObject and maybe external vertices (this I have to check). Invalid geometry index in Constraints causes that the returned set of constraints is empty (see inlined getValue in PropertConstraintList.h).

IMO, this architecture has three problems:
1) It updates the ViewProvider when it does not have to, as mid-undo-transaction, there is a high change the data used for the update is invalid (this causes the crash).
2) It updates the ViewProvider when it does not need to, as it is enough to update it once when all properties are processed.
3) It abuses recomputes, when no recompute should be needed.

The solution

The solution I have thought to adopt for this issue, is to delay ViewProvider update until all the transactions are performed. This can reasonably be done by blocking updateData using isPerformingTransaction():

Code: Select all

    if ( edit && !getSketchObject()->getDocument()->isPerformingTransaction() &&
         (prop == &(getSketchObject()->Geometry) || prop == &(getSketchObject()->Constraints))) {

This solution requires that an update of the view provider is generated when the transaction is over (i.e. after undoing).

Although this may not be the final solution (see below), updating via Gui::Document:signalUndoDocument and signalRedoDocument, which are even already connected in the current code, is a possible solution to this problem.

Looking into this, made the following issue arise:

ViewProviderSketch::slotUndoDocument and slotRedoDocument and the recomputes

The current implementation (current master) is rather worthless. It solve() or recompute(). When solve() it is worthless because the validgeometrykeys of the constraints are not updated after paste (see SketchObject issue below). When recomputing is worthless because during undo and redo recomputes (as the ones used) are inhibited.

However these functions could be leveraged (and are leveraged in the solution above).

Looking into this undo signals, we do have more things connected to them:

Issue 3: TaskDlgEditSketch::slotUndoDocument and slotRedoDocument

This is the real reason why things currently "work". These slots, which are also connected to Gui::Document:signalUndoDocument and signalRedoDocument, directly call recomputeFeature() to get away with the recompute inhibit.

This recompute generates valid geometry keys (via acceptGeometry()). However, this recompute should be unnecessary if the rest worked appropriately, at least when the Auto-Update is not checked.

SketchObject and why the solution of the branch above is not what it should be

When the PropertyConstraintList::Paste is executed, there is no update of the "validGeometrykeys" of PropertyConstraintList, until two executes are run (a first one to execute acceptGeometry at the end, while having solved with empty constraints due to invalid geometry keys, and the second one actually solving the now valid constraints).

A separate issue is that the vertexIndex may also be invalid after undo/redo.

The branch above fixes this problem calling checkgeometry() from ViewProviderSketch when the undo signal is received.

However, this is not really the place for calling this function. The object that should be responsible is App::SketchObject, not its ViewProvider (also think of FreeCADCmd).

However, for SketchObject to take this responsibility (which is why I think should be the real solution), it needs to be notified when the undo/redo transaction is finished. Here it where my problems come.

I see two options:
a) use the signal App::Document::signalUndo and signalRedo
b) create a virtual function in DocumentObject::onUndoRedoFinished similar to the existing void DocumentObject::onDocumentRestored()

The possible solution a) has these problems:
1) The subscribed document objects would receive an undo signal, even if it is not intended for them (the undo relates to another object).
2) It requires creative connection and disconnection of signals, as a DocumentObject can have a null Document (for example in the constructor) or may change Document. I am not sure yet where the "good spots" to perform this management is.

Option b) (which implementation is started in the last commit of the branch above, but is not used when running yet) would require that a list of the documentobjects affected by the properties is retrievable before or after the transaction, so that onUndoRedoFinished can be called for them (thereby avoiding flooding all the document with calls). The object applying the transaction is Transaction::apply containing a list of TransactionObject, which can be used to get the document objects.

This option b) requires some changes, so I think it is better to ask before. Maybe there is a better, faster, cleaner solution I am not seeing

EDIT: actually b) together with an Status bit in document object might work without too much change... working on it.

[abdullah]

Maybe we can think of a controlling instance that observes the two properties when a transaction is performed and notifies the sketch viewprovider when the transaction is finished.

Please take a look to this PR when you have time:
https://github.com/FreeCAD/FreeCAD/pull/3610

I have used a status bit so that documentobjects can request on demand a notification after an undo/redo transaction is completed. This solves many problems relating to inconsistent sketchobject, unnecessary viewprovider updates and unnecessary recomputes.

However, you may come up with a better solution.

[wmayer]

Please take a look to this PR when you have time:
https://github.com/FreeCAD/FreeCAD/pull/3610

I have used a status bit so that documentobjects can request on demand a notification after an undo/redo transaction is completed. This solves many problems relating to inconsistent sketchobject, unnecessary viewprovider updates and unnecessary recomputes.

However, you may come up with a better solution.

I have checked the PR and to me it looks very good. So, I will merge it.

But as you know me I always find a fly in the ointment The PR fixes the crash for standard users but it's still possible to make it easily crash via Python:
After loading the above test file run this Python code

Code: Select all

s=App.ActiveDocument.Sketch
c=s.Constraints

Remove the geometries of the sketch and run this

Code: Select all

s.Constraints=c

Of course it's the same call stack as shown above. So, additionally to the proposed changes in ViewProviderSketch::rebuildConstraintsVisual we must check the returned pointers of SketchObject::getGeometry() if one of them is null.

[wmayer]

git commit 76f818d017f6