Crash with "Restore previous session" when file does not exist

[ajschult]

With current git master, I attempted to use the Recent->Restore Session menu item when the previously-loaded file did not exist.  The gui first informed me that the file did not exist ("file.mp4" does not exist) and then crashed:

Assert failed :ref<_segments.getNbRefVideos()
at line 690, file /home/andrew/build/avidemux/avidemux2/avidemux/qt4/common/ADM_editor/src/ADM_edRender.cpp

Code Select Expand


#0  0x00007ffff54c1e35 in raise () from /lib64/libc.so.6
#1  0x00007ffff54ac895 in abort () from /lib64/libc.so.6
#2  0x000000000044a738 in FatalFunctionQt(char const*, char const*) [clone .cold] ()
#3  0x00007ffff72e2dfe in ADM_backTrack ()
   from /home/andrew/build/avidemux/avidemux2-clean/avidemux2/install/usr/lib64/libADM_core6.so
#4  0x000000000046cf98 in ADM_Composer::addSegment(unsigned int, unsigned long, unsigned long) ()
#5  0x00007fffe217209c in zzpy_addSegment(tp_vm*) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#6  0x00007fffe215ef87 in _tp_dcall(tp_vm*, tp_obj (*)(tp_vm*)) ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#7  0x00007fffe215f344 in _tp_tcall(tp_vm*, tp_obj) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#8  0x00007fffe2163464 in tp_call(tp_vm*, tp_obj, tp_obj) [clone .localalias] ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#9  0x00007fffe216a223 in tp_step(tp_vm*) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#10 0x00007fffe2162fdd in _tp_run(tp_vm*, int) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#11 0x00007fffe21630d9 in tp_run(tp_vm*, int) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#12 0x00007fffe216b5b9 in tp_exec_(tp_vm*) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#13 0x00007fffe215ef87 in _tp_dcall(tp_vm*, tp_obj (*)(tp_vm*)) ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#14 0x00007fffe215f344 in _tp_tcall(tp_vm*, tp_obj) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#15 0x00007fffe2163464 in tp_call(tp_vm*, tp_obj, tp_obj) [clone .localalias] ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#16 0x00007fffe216a223 in tp_step(tp_vm*) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#17 0x00007fffe2162fdd in _tp_run(tp_vm*, int) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#18 0x00007fffe21630d9 in tp_run(tp_vm*, int) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#19 0x00007fffe2163352 in tp_call(tp_vm*, tp_obj, tp_obj) [clone .localalias] ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#20 0x00007fffe216ac80 in tp_ez_call(tp_vm*, char const*, char const*, tp_obj) ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#21 0x00007fffe216b0c9 in _tp_import(tp_vm*, tp_obj, tp_obj, tp_obj) ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#22 0x00007fffe216b1ab in tp_import(tp_vm*, char const*, char const*, void*, int) ()
   from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#23 0x00007fffe2179b3a in PythonEngine::runScriptFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, IScriptEngine::RunMode) () from /usr/lib64/ADM_plugins6/scriptEngines/libADM_script_tinyPy.so
#24 0x000000000045008d in parseScript(IScriptEngine*, char const*, IScriptEngine::RunMode) [clone .isra.0] ()
#25 0x0000000000453285 in A_checkSavedSession(bool) ()
#26 0x0000000000453c89 in HandleAction(Action) ()
#27 0x00000000004abb35 in MainWindow::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) ()
#28 0x00007ffff58f8d7b in QMetaObject::activate(QObject*, int, int, void**) () from /lib64/libQt5Core.so.5
#29 0x00000000004ab96e in MainWindow::actionSignal(Action) ()
#30 0x000000000048c9cd in MainWindow::searchMenu(QAction*, MenuEntry*, int) ()
#31 0x00000000004abd67 in MainWindow::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) ()
#32 0x00007ffff58f8d7b in QMetaObject::activate(QObject*, int, int, void**) () from /lib64/libQt5Core.so.5
#33 0x00007ffff64d2916 in QMenu::triggered(QAction*) () from /lib64/libQt5Widgets.so.5
#34 0x00007ffff64d947c in ?? () from /lib64/libQt5Widgets.so.5
#35 0x00007ffff64df692 in ?? () from /lib64/libQt5Widgets.so.5
#36 0x00007ffff58f8d7b in QMetaObject::activate(QObject*, int, int, void**) () from /lib64/libQt5Core.so.5
#37 0x00007ffff634c476 in QAction::triggered(bool) () from /lib64/libQt5Widgets.so.5
#38 0x00007ffff634eb32 in QAction::activate(QAction::ActionEvent) () from /lib64/libQt5Widgets.so.5
#39 0x00007ffff64d4472 in ?? () from /lib64/libQt5Widgets.so.5
#40 0x00007ffff64dbbbe in ?? () from /lib64/libQt5Widgets.so.5
#41 0x00007ffff64dcbe2 in QMenu::mouseReleaseEvent(QMouseEvent*) () from /lib64/libQt5Widgets.so.5
#42 0x00007ffff639516e in QWidget::event(QEvent*) () from /lib64/libQt5Widgets.so.5
#43 0x00007ffff64df1db in QMenu::event(QEvent*) () from /lib64/libQt5Widgets.so.5
#44 0x00007ffff6352af6 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /lib64/libQt5Widgets.so.5
#45 0x00007ffff635c0d3 in QApplication::notify(QObject*, QEvent*) () from /lib64/libQt5Widgets.so.5
#46 0x00007ffff58ceae8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /lib64/libQt5Core.so.5
#47 0x00007ffff635b1b7 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) () from /lib64/libQt5Widgets.so.5
#48 0x00007ffff63b1834 in ?? () from /lib64/libQt5Widgets.so.5
#49 0x00007ffff63b3d3c in ?? () from /lib64/libQt5Widgets.so.5
#50 0x00007ffff6352af6 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /lib64/libQt5Widgets.so.5
#51 0x00007ffff635be80 in QApplication::notify(QObject*, QEvent*) () from /lib64/libQt5Widgets.so.5
#52 0x00007ffff58ceae8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /lib64/libQt5Core.so.5
#53 0x00007ffff5dc3783 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) ()
   from /lib64/libQt5Gui.so.5
#54 0x00007ffff5dc4fab in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) () from /lib64/libQt5Gui.so.5
#55 0x00007ffff5da14fb in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /lib64/libQt5Gui.so.5
#56 0x00007fffe2cbdbee in ?? () from /lib64/libQt5XcbQpa.so.5
#57 0x00007ffff4663ecd in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#58 0x00007ffff4664260 in g_main_context_iterate.isra () from /lib64/libglib-2.0.so.0
#59 0x00007ffff4664303 in g_main_context_iteration () from /lib64/libglib-2.0.so.0
#60 0x00007ffff5923bd5 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /lib64/libQt5Core.so.5
#61 0x00007ffff58cd9eb in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /lib64/libQt5Core.so.5
#62 0x00007ffff58d5726 in QCoreApplication::exec() () from /lib64/libQt5Core.so.5
#63 0x0000000000494cdf in UI_RunApp() ()
#64 0x000000000045a5d9 in startAvidemux(int, char**) ()
#65 0x000000000044d2f0 in main ()


The lastEdit.py script doesn't check if the file load actually succeeded and addSegment objects to being called without actual underlying data.

[eumagga0x2a]

It would not be enough to check that the file still exists. It would be necessary to pre-parse each script to split it in expressions instead of delegating all this to a script engine. The current design provides no means to stop execution after a failure, thus assert as the lesser evil.

[ajschult]

No.  In fact,

Code Select Expand


adm = Avidemux()
rv = adm.loadVideo("/home/andrew/bogus.flv")
if rv == 1:
  adm.clearSegments()
  adm.addSegment(0, 0, 17768000)
  ...


works fine.  It complains about the missing file and does not crash.

[eumagga0x2a]

As long as you implement the necessary return value check in python, yes, this works. But the problem is with scripts which don't do that, which are all existing auto-generated scripts including session saving.

And now imagine that a failure occurs in a call somewhere later on. What should we do then? Only stop there? Close the last appended video? Close all videos and invalidate the session?

It is also absolutely legitimate not to loadVideo() in a script at all and start with addSegment() right away which currently doesn't check whether its arguments match the given video... It is a big can of worms.

The easiest path would be to patch PythonScriptWriter.cpp to include rv checks for loadVideo() and to add input value checks to addSegment(), but it will still remain a patchwork (no pun intended) and will make scripts harder to read for a layman.

[ajschult]

It's hard to imagine anything worse than crashing because a file disappeared.  It'd be great if it was impossible to crash the app from python, but I can accept that it may be too much work.  But that's a long way away from "don't autogenerate scripts that crash".

If you don't want to write a more complicated script (is return checking really that complicated?), admPyClass.pl could be taught to pay attention to return codes from functions like loadVideo and raise exceptions.  It'd be more work, but it would also catch a wider class of problems.

[eumagga0x2a]

I wholeheartedly agree UX-wise. Just too many construction areas at once.