From a34c877fd0e01e2a0fd3860b6e9507fee61c9fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ximo=20Casta=C3=B1eda?= Date: Wed, 8 Feb 2023 12:27:04 +0100 Subject: [PATCH] app_server: FontStyle lifecycle FontFamily is babysitted by the FontManager. It doesn't own FontStyle references, there's no place in it to release them. FontManager owns a reference to the FontStyle, and it is in fStyleHashTable. Putting a style there acquires a new reference, so we can release the one from the new style. Removing a style from the map releases the reference. We need to clear the map before shutting down FreeType to get rid of our last references and let the styles die now instead of afterwards to avoid double freeing the faces. These solve the new crash from the previous patch. It didn't crash before because even after the map was destroyed there were still dangling references to the styles. On removing a user font, the style will remove itself from the family through the manager if that was the last reference, and the manager will remove and delete the family if it has no more styles. Change-Id: I460ff830fa8a8a5adb90dc8ea12120e1e50a5912 Reviewed-on: https://review.haiku-os.org/c/haiku/+/6052 Reviewed-by: waddlesplash --- src/servers/app/ServerApp.cpp | 7 +------ src/servers/app/font/AppFontManager.cpp | 17 +++-------------- src/servers/app/font/FontFamily.cpp | 2 -- src/servers/app/font/FontManager.cpp | 8 ++++++-- src/servers/app/font/GlobalFontManager.cpp | 3 +-- 5 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/servers/app/ServerApp.cpp b/src/servers/app/ServerApp.cpp index e6c42e22a9..48b9bc5306 100644 --- a/src/servers/app/ServerApp.cpp +++ b/src/servers/app/ServerApp.cpp @@ -1766,12 +1766,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) status_t status = B_OK; AutoLocker fontLock(fAppFontManager); - FontStyle* style = fAppFontManager->GetStyle(familyID, styleID); - - if (style != NULL) { - status = fAppFontManager->RemoveUserFont(familyID, styleID); - } else - status = B_BAD_VALUE; + status = fAppFontManager->RemoveUserFont(familyID, styleID); fLink.StartMessage(status); fLink.Flush(); diff --git a/src/servers/app/font/AppFontManager.cpp b/src/servers/app/font/AppFontManager.cpp index a10424bd05..60cce7db1c 100644 --- a/src/servers/app/font/AppFontManager.cpp +++ b/src/servers/app/font/AppFontManager.cpp @@ -95,6 +95,7 @@ AppFontManager::_AddUserFont(FT_Face face, node_ref nodeRef, const char* path, styleID = style->ID(); fStyleHashTable.Put(FontKey(familyID, styleID), style); + style->ReleaseReference(); return B_OK; } @@ -160,19 +161,7 @@ AppFontManager::RemoveUserFont(uint16 familyID, uint16 styleID) ASSERT(IsLocked()); FontKey fKey(familyID, styleID); - FontStyle* styleRef = fStyleHashTable.Get(fKey); - fStyleHashTable.Remove(fKey); + FontStyle* styleRef = fStyleHashTable.Remove(fKey); - FontFamily* family = styleRef->Family(); - bool removed = family->RemoveStyle(styleRef, this); - - if (!removed) - syslog(LOG_DEBUG, "AppFontManager::RemoveUserFont style not removed from family\n"); - - fFamilies.RemoveItem(family); - delete family; - - styleRef->ReleaseReference(); - - return B_OK; + return styleRef != NULL ? B_OK : B_BAD_VALUE; } diff --git a/src/servers/app/font/FontFamily.cpp b/src/servers/app/font/FontFamily.cpp index 1af8024fdd..6d063ebada 100644 --- a/src/servers/app/font/FontFamily.cpp +++ b/src/servers/app/font/FontFamily.cpp @@ -78,8 +78,6 @@ FontFamily::~FontFamily() // we remove us before deleting the style, so that the font manager // is not contacted to remove the style from us style->_SetFontFamily(NULL, -1); - - style->ReleaseReference(); } } diff --git a/src/servers/app/font/FontManager.cpp b/src/servers/app/font/FontManager.cpp index 4b307b4257..221e4c7e34 100644 --- a/src/servers/app/font/FontManager.cpp +++ b/src/servers/app/font/FontManager.cpp @@ -69,6 +69,8 @@ FontManagerBase::~FontManagerBase() for (int32 i = fFamilies.CountItems(); i-- > 0;) delete fFamilies.ItemAt(i); + fStyleHashTable.Clear(); + if (fHasFreetypeLibrary == true) FT_Done_FreeType(gFreeTypeLibrary); } @@ -306,8 +308,10 @@ FontManagerBase::RemoveStyle(FontStyle* style) debugger("style removed but still available!"); if (family->RemoveStyle(style) - && family->CountStyles() == 0) + && family->CountStyles() == 0) { fFamilies.RemoveItem(family); + delete family; + } } @@ -320,4 +324,4 @@ FontManagerBase::_FindFamily(const char* name) const FontFamily family(name, 0); return const_cast(fFamilies.BinarySearch(family, compare_font_families)); -} \ No newline at end of file +} diff --git a/src/servers/app/font/GlobalFontManager.cpp b/src/servers/app/font/GlobalFontManager.cpp index cddbdbf98f..3fe6d15182 100644 --- a/src/servers/app/font/GlobalFontManager.cpp +++ b/src/servers/app/font/GlobalFontManager.cpp @@ -484,8 +484,6 @@ GlobalFontManager::_RemoveStyle(font_directory& directory, FontStyle* style) directory.revision++; fStyleHashTable.Remove(FontKey(style->Family()->ID(), style->ID())); - - style->ReleaseReference(); } @@ -741,6 +739,7 @@ GlobalFontManager::_AddFont(font_directory& directory, BEntry& entry) directory.styles.AddItem(style); fStyleHashTable.Put(FontKey(style->Family()->ID(), style->ID()), style); + style->ReleaseReference(); if (directory.AlreadyScanned()) directory.revision++;