Deadlock when switching skin while announcement is displayed

[neo1973]

Bug report

Describe the bug

When switching skins from settings while an announcement is shown a deadlock occurs. Stack traces of all threads, the locking threads are #1 and #17:

Stack trace

Expected Behavior

Skin switches.

Actual Behavior

Deadlock.

Possible Fix

Don't know. The easy ones like reducing the scope of the lock in CDirectoryProvider::Reset() may introduce race conditions.

To Reproduce

Steps to reproduce the behavior:

1. Start a library scan or something else that puts announcements on the screen
2. Go to settings and switch to a different skin

Debuglog

Unfortunatly an other bug prevents me from creating a debug log at the moment, but I hope the stack traces are sufficient to see what's going on.

Your Environment

• Android

• iOS

• tvOS

• Linux

• OSX

• Windows

• Windows UWP

• Operating system version/name: Arch Linux, recently updated

• Kodi version: current master, commit 9c3a68f

note: Once the issue is made we require you to update it with new information or Kodi versions should that be required.
Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

[ksooo]

@neo1973 shouldn't this do the trick?

diff --git a/xbmc/interfaces/AnnouncementManager.cpp b/xbmc/interfaces/AnnouncementManager.cpp
index abd1ca8c94..ce118cc88c 100644
--- a/xbmc/interfaces/AnnouncementManager.cpp
+++ b/xbmc/interfaces/AnnouncementManager.cpp
@@ -151,11 +151,14 @@ void CAnnouncementManager::DoAnnounce(AnnouncementFlag flag,
 {
   CLog::Log(LOGDEBUG, LOGANNOUNCE, "CAnnouncementManager - Announcement: {} from {}", message, sender);
 
-  std::unique_lock<CCriticalSection> lock(m_announcersCritSection);
-
-  // Make a copy of announcers. They may be removed or even remove themselves during execution of IAnnouncer::Announce()!
+  m_announcersCritSection.lock();
 
+  // Make a copy of announcers. They may be removed or even remove
+  // themselves during execution of IAnnouncer::Announce()!
   std::vector<IAnnouncer *> announcers(m_announcers);
+
+  m_announcersCritSection.unlock();
+
   for (unsigned int i = 0; i < announcers.size(); i++)
     announcers[i]->Announce(flag, sender, message, data);
 }

[neo1973]

This would fix the deadlock but wouldn't we potentially access a dangling pointer instead? In the stack trace CDirectoryProvider removes itself from the CAnnouncementManager because it's destroyed but the copied announcers vector still contains the now invalid pointer.

[ksooo]

because it's destroyed but the copied announcers vector still contains the now invalid pointer.

Right. :-/

[ksooo]

Fixed with #22459 (master), #22460 (Nexus)