Delete timeline causes crash on Windows

[wolfseifert]

The following code line

connect(timeline, &QTimeLine::finished, [timeline] { delete timeline; });

in ui/RepoView.cpp:893 (same in ui/MainWindow.cpp:204) causes my private fork of Gittyup to crash on Windows. The timeline is accessed after deletion and has therefore invalid pointer values.

Here is a short reproducer:

#include <QtWidgets>

auto main(int argc, char* argv[]) -> int {
  auto app = QApplication{argc, argv};
  auto win = QMainWindow{};
  auto progressBar = new QProgressBar{};
  progressBar->setRange(0, 100);

  auto timeLine = new QTimeLine{1000, &win};
  timeLine->setFrameRange(0, 100);
  win.connect(timeLine, &QTimeLine::frameChanged, progressBar, &QProgressBar::setValue);
  win.connect(timeLine, &QTimeLine::finished, [timeLine] {
    delete timeLine;
  });
  timeLine->start();

  win.setCentralWidget(progressBar);
  win.show();
  return QApplication::exec();
}

Fix: Remove these lines!

[Murmele]

Maybe using deleteLater() instead

[wolfseifert]

Maybe this will prevent the crash, maybe not. The delete timeLine already happens on the main-thread, so deleteLater could happen instantly.

I found nothing in the Qt-Assistant about explicitly deleting a created QTimeLine, so doing nothing (i.e. removing the two lines of code mentioned above) is probably the better option. Better to have a (small) memory leak than an application crash.

[Murmele]

Are you able to reproduce the crash? Can you try out with deleteLater?

[wolfseifert]

I already tried it before my last comment:

It does not happen with deleteLate on windows, it never happenes on linux!

But as long as I can not find a hint in the documentation when to delete QTimeLine safely, I would be cautious and just don't do it!

[wolfseifert]

A debug session later:

The line with delete timeLine; is done during timer-event handling, which causes a crash when going back up the stack.

Replacing it with timeLine.deleteLater(); during the timer-event handling, the stack is upwinded successfully (because the timeLine has not been deleted yet) and the the actual delete timeLine; happens on another deferred-deletion-event handling.

So this works. Unfortunately I could not find any documentation on this ("How to delete safely a QTimeLine") .

[Murmele]

Thanks for testing! Maybe it can be handled as any other QObject and therefore deleteLater() works fine. Neverthless I wanna get rid of the memory instead of having a memory leakage. Can you make a pull request?

[Murmele]

fixed with #693