From fed5e6126b1340b5f40dee2570f12c0c651f71da Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Sun, 19 Jan 2014 12:12:57 +0100 Subject: [PATCH] LocaleRoster: race condition on catalog loading. Using a vint32 isn't enough to safely decide if the catalog is initialized or not. Use init_once features instead. --- headers/os/locale/LocaleRoster.h | 4 +++- src/kits/locale/CatalogStub.cpp | 8 +++++--- src/kits/locale/LocaleRoster.cpp | 32 +++++++++++++++++++------------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/headers/os/locale/LocaleRoster.h b/headers/os/locale/LocaleRoster.h index fab246939c..e4ecb1335f 100644 --- a/headers/os/locale/LocaleRoster.h +++ b/headers/os/locale/LocaleRoster.h @@ -6,6 +6,8 @@ #define _LOCALE_ROSTER_H_ +#include + #include #include @@ -98,7 +100,7 @@ protected: private: static BCatalog* _GetCatalog(BCatalog* catalog, - vint32* catalogInitStatus); + int32* catalogInitStatus); status_t _PrepareCatalogEntry(const entry_ref& ref, BString& signature, BString& context, diff --git a/src/kits/locale/CatalogStub.cpp b/src/kits/locale/CatalogStub.cpp index 03a576fba2..0e07bbc551 100644 --- a/src/kits/locale/CatalogStub.cpp +++ b/src/kits/locale/CatalogStub.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2010, Adrien Destugues . + * Copyright 2010-2014, Adrien Destugues . * Distributed under the terms of the MIT License. */ @@ -7,9 +7,11 @@ #include #include +#include + static BCatalog sCatalog; -static vint32 sCatalogInitOnce = false; +static int32 sCatalogInitOnce = INIT_ONCE_UNINITIALIZED; BCatalog* @@ -28,7 +30,7 @@ BLocaleRoster::GetCatalog() namespace BPrivate{ void ForceUnloadCatalog() { - sCatalogInitOnce = false; + sCatalogInitOnce = INIT_ONCE_UNINITIALIZED; } } diff --git a/src/kits/locale/LocaleRoster.cpp b/src/kits/locale/LocaleRoster.cpp index c296ab64d8..115b3c8bff 100644 --- a/src/kits/locale/LocaleRoster.cpp +++ b/src/kits/locale/LocaleRoster.cpp @@ -32,6 +32,7 @@ #include #include +#include // ICU includes #include @@ -515,16 +516,10 @@ BLocaleRoster::GetLocalizedFileName(BString& localizedFileName, } -BCatalog* -BLocaleRoster::_GetCatalog(BCatalog* catalog, vint32* catalogInitStatus) +static status_t +_InitializeCatalog(void* param) { - // This function is used in the translation macros, so it can't return a - // status_t. Maybe it could throw exceptions ? - - if (*catalogInitStatus == true) { - // Catalog already loaded - nothing else to do - return catalog; - } + BCatalog* catalog = (BCatalog*)param; // figure out image (shared object) from catalog address image_info info; @@ -540,13 +535,24 @@ BLocaleRoster::_GetCatalog(BCatalog* catalog, vint32* catalogInitStatus) } if (!found) - return catalog; + return B_NAME_NOT_FOUND; - // load the catalog for this mimetype and return it to the app + // load the catalog for this mimetype entry_ref ref; - if (BEntry(info.name).GetRef(&ref) == B_OK && catalog->SetTo(ref) == B_OK) - *catalogInitStatus = true; + if (BEntry(info.name).GetRef(&ref) == B_OK && catalog->SetTo(ref) == B_OK); + return B_OK; + + return B_ERROR; +} + +BCatalog* +BLocaleRoster::_GetCatalog(BCatalog* catalog, int32* catalogInitStatus) +{ + // This function is used in the translation macros, so it can't return a + // status_t. Maybe it could throw exceptions ? + + __init_once(catalogInitStatus, _InitializeCatalog, catalog); return catalog; }