FEB
20
2021

unique_ptr difference between libstdc++ and libc++ crashes your application

Thanks to the KDE FreeBSD CI, which runs our code on top of libc++, we discovered an interesting difference between libstdc++ and libc++'s implementation of unique_ptr. This is quite unexpected, and the actual result for users is even more unexpected: it can lead to crashes in specific situations. This happens when a widget -- using unique_ptr for its d pointer, as is customary these days -- installs an event filter. That event filter will be triggered during destruction of child widgets (at least for the QEvent::Destroy event, I've also seen it with QEvent::Leave events for instance). And, depending on how the event filter is written, it might use the d pointer of the widget, possibly before checking the event type. That's where it gets interesting: the libc++ implementation of unique_ptr sets it to null *before* calling the destructor (because it's implemented in terms of reset(nullptr);. In libstdc++ however, unique_ptr's destructor just calls the destructor, its value remains valid during destruction.

Here's a testcase of the crash, extracted from the KIO code for which you can see the fix here. It works with gcc/libstdc++, and crashes with clang/libc++, because MyWindow::eventFilter() is using a d pointer that is null already.
In this particular case the fix is easy, testing the event type before anything else. That's good practice anyway, for performance reasons. But there are other cases, where the situation was a bit different and the solution was to remove the event filter upon destruction.

I like unique_ptr, but beware of the interaction with event filters...

Comments

Thanks for the heads up.

Minor fix: In the second paragraph, I think you mean clang/libc++, not clang/libstdc++?


By Elvis Stansvik at Sat, 02/20/2021 - 17:12

Thanks, fixed.


By David Faure at Sat, 02/20/2021 - 17:39

There's been some Reddit discussion of this post, which culminates in:

  • Our behavior was well-defined per language rules (there was debate on this)
  • But our behavior is undefined at a library level. See https://cplusplus.github.io/LWG/issue2224. That is, the C++ standard library does not permit references to members of library types during construction or destruction even if the C++ language otherwise permits it.

User libraries are not so constrained. We may want to deliberately use Qt or custom types for that reason if doing hairy work in our own constructors/destructors.


By Michael Pyne at Sat, 02/20/2021 - 18:55

So, the destructor of MyWindow is called, it destroys the d pointer and then the QObject's destructor destroys the children which triggers MyWindow's eventFilter that tries to access the d pointer? If that is the case, I'm surprised it worked with libstdc++. :)

I've seen a similar issue with a connection that was not yet disconnected because QObject was still not destroyed, but the ~QObject destructor started which meant that "this" that the slot (lambda) accessed is no longer a SubClassOfQObject. The issue was caught by ASAN/USAN.

Removing connections (storing the connection as a member variable) and event filters in proper destructors should be a must.


By Ivan at Sat, 02/20/2021 - 19:16