Enable testing for whether a calendar registered its primary name

In registerAlias(), return true if this instance is already registered
with the given name.

Previously there was no way for a QCalendarBackend to tell whether its
primary name registration had succeeded, during instantiation (other
than by devious hackery using a QCalendar instance with the name and
some form of back-channel in the instance).

Use this in backendFromEnum() to catch cases in which (e.g. due to a
race condition) a new instance isn't the one that got registered.

Pick-to: 6.0 5.15
Change-Id: I468ac364a68bf3574cd7f8b8b1e672d8fd969111
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Edward Welbourne 2020-10-05 13:39:45 +02:00
parent 94c3c7a491
commit 2b7c74d5ff
2 changed files with 95 additions and 25 deletions

View File

@ -96,17 +96,20 @@ struct Registry {
bool registerName(QCalendarBackend *calendar, const QString &name) bool registerName(QCalendarBackend *calendar, const QString &name)
{ {
if (byName.find(name) != byName.end()) { Q_ASSERT(!name.isEmpty());
qWarning() << "Calendar name" << name const auto found = byName.find(name);
<< "is already taken, new calendar will not be registered."; if (found != byName.end()) {
return false; // Re-registering a calendar with a name it has already is OK (and
// can be used to test whether its constructor successfully
// registered its primary name).
return found.value() == calendar;
} }
byName.insert(name, calendar); byName.insert(name, calendar);
return true; return true;
} }
void addCalendar(QCalendarBackend *calendar, const QString &name, QCalendar::System id) void addCalendar(QCalendarBackend *calendar, const QString &name, QCalendar::System id)
{ {
if (!registerName(calendar, name)) if (name.isEmpty() || !registerName(calendar, name))
return; return;
Q_ASSERT(byId.size() >= size_t(id)); Q_ASSERT(byId.size() >= size_t(id));
if (id == QCalendar::System::User) { if (id == QCalendar::System::User) {
@ -148,28 +151,44 @@ Q_GLOBAL_STATIC(Registry, calendarRegistry);
static const QCalendarBackend *backendFromEnum(QCalendar::System system) static const QCalendarBackend *backendFromEnum(QCalendar::System system)
{ {
QCalendarBackend *backend = nullptr;
switch (system) { switch (system) {
case QCalendar::System::Gregorian: case QCalendar::System::Gregorian:
return new QGregorianCalendar; backend = new QGregorianCalendar;
break;
#ifndef QT_BOOTSTRAPPED #ifndef QT_BOOTSTRAPPED
case QCalendar::System::Julian: case QCalendar::System::Julian:
return new QJulianCalendar; backend = new QJulianCalendar;
break;
case QCalendar::System::Milankovic: case QCalendar::System::Milankovic:
return new QMilankovicCalendar; backend = new QMilankovicCalendar;
break;
#endif #endif
#if QT_CONFIG(jalalicalendar) #if QT_CONFIG(jalalicalendar)
case QCalendar::System::Jalali: case QCalendar::System::Jalali:
return new QJalaliCalendar; backend = new QJalaliCalendar;
break;
#endif #endif
#if QT_CONFIG(islamiccivilcalendar) #if QT_CONFIG(islamiccivilcalendar)
case QCalendar::System::IslamicCivil: case QCalendar::System::IslamicCivil:
return new QIslamicCivilCalendar; backend = new QIslamicCivilCalendar;
break;
#else // When highest-numbered system isn't enabled, ensure we have a case for Last: #else // When highest-numbered system isn't enabled, ensure we have a case for Last:
case QCalendar::System::Last: case QCalendar::System::Last:
#endif #endif
case QCalendar::System::User: case QCalendar::System::User:
Q_UNREACHABLE(); Q_UNREACHABLE();
} }
if (!backend)
return backend;
const QString name = backend->name();
// Check for successful registration:
if (calendarRegistry->registerName(backend, name))
return backend;
delete backend;
const auto found = calendarRegistry->byName.find(name);
if (found != calendarRegistry->byName.end())
return found.value();
return nullptr; return nullptr;
} }
@ -186,11 +205,12 @@ static const QCalendarBackend *backendFromEnum(QCalendar::System system)
implemented. On construction, the backend is registered with its primary implemented. On construction, the backend is registered with its primary
name. name.
A backend may also be registered with aliases, where the calendar is known A backend, once successfully registered with its primary name, may also be
by several names. Registering with the name used by CLDR (the Unicode registered with aliases, where the calendar is known by several
consortium's Common Locale Data Repository) is recommended, particularly names. Registering with the name used by CLDR (the Unicode consortium's
when interacting with third-party software. Once a backend is registered for Common Locale Data Repository) is recommended, particularly when interacting
a name, QCalendar can be constructed using that name to select the backend. with third-party software. Once a backend is registered for a name,
QCalendar can be constructed using that name to select the backend.
Each calendar backend must inherit from QCalendarBackend and implement its Each calendar backend must inherit from QCalendarBackend and implement its
pure virtual methods. It may also override some other virtual methods, as pure virtual methods. It may also override some other virtual methods, as
@ -210,22 +230,67 @@ static const QCalendarBackend *backendFromEnum(QCalendar::System system)
*/ */
/*! /*!
Constructs the calendar and registers it under \a name using \a id. Constructs the calendar and registers it under \a name using \a system.
On successful registration, the calendar backend registry takes over
ownership of the instance and shall delete it on program exit in the course
of the registry's own destruction. The instance can determine whether it was
successfully registered by calling registerAlias() with the same \a name it
passed to this base-class constructor. If that returns \c false, the
instance has not been registered, QCalendar cannot use it, it should not
attempt to register any other aliases and the code that instantiated the
backend is responsible for deleting it.
The \a system is optional and should only be passed by built-in
implementations of the standard calendars documented in \l
QCalendar::System. Custom backends should not pass \a system.
Only one backend instance should ever be registered for any given \a system:
in the event of a backend being created when one with the same \a system
already exists, the new backend is not registered. The \a name passed with a
\a system (other than \l{QCalendar::System}{User}) must be the \c{name()} of
the backend constructed.
The \a name must be non-empty and unique; after one backend has been
registered for a name or alias, no other backend can be registered with that
name. The presence of another backend registered with the same name may mean
the backend is redundant, as the system already has a backend to handle the
given calendar type.
\note \c{QCalendar(name).isValid()} will return true precisely when the
given \c name is in use already. This can be used as a test before
instantiating a backend with the given \c name.
\sa calendarId(), calendarSystem(), registerAlias()
*/ */
QCalendarBackend::QCalendarBackend(const QString &name, QCalendar::System id) QCalendarBackend::QCalendarBackend(const QString &name, QCalendar::System system)
{ {
calendarRegistry->addCalendar(this, name, id); Q_ASSERT(!name.isEmpty());
calendarRegistry->addCalendar(this, name, system);
} }
/*! /*!
Destroys the calendar. Destroys the calendar.
Never call this from user code. Each calendar backend, once instantiated, Client code should only call this if instantiation failed to register the
shall exist for the lifetime of the program. Its destruction is taken care backend, as revealed by the instanee failing to registerAlias() with the
of by destruction of the registry of calendar backends and their names. name it passed to this base-class's constructor. Only a backend that fails
to register can safely be deleted; and the client code that instantiated it
is indeed responsible for deleting it.
Once a backend has been successfully registered, there may be QCalendar
instances using it; deleting it while they still reference it would lead to
undefined behavior. Such a backend shall be deleted when the calendar
backend registry is deleted on program exit; the registry takes over
ownership of the instance on successful registration.
\sa registerAlias()
*/ */
QCalendarBackend::~QCalendarBackend() QCalendarBackend::~QCalendarBackend()
{ {
// Either the registry is destroying itself, in which case it takes care of
// dropping any references to this, or this never got registered, so there
// is no need to tell the registry to forget it.
} }
/*! /*!
@ -610,15 +675,20 @@ QStringList QCalendarBackend::availableCalendars()
its name will be included in the list of available calendars and the its name will be included in the list of available calendars and the
calendar can be instantiated by name. calendar can be instantiated by name.
Returns \c false if the given \a name is already in use, otherwise it Returns \c false if the given \a name is already in use by a different
registers this calendar backend and returns \c true. backend or \c true if this calendar is already registered with this
name. (This can be used, with its primary name, to test whether a backend's
construction successfully registered it.) Otherwise it registers this
calendar backend for this name and returns \c true.
\sa availableCalendars(), fromName() \sa availableCalendars(), fromName()
*/ */
bool QCalendarBackend::registerAlias(const QString &name) bool QCalendarBackend::registerAlias(const QString &name)
{ {
if (calendarRegistry.isDestroyed()) if (calendarRegistry.isDestroyed() || name.isEmpty())
return false; return false;
// Constructing this accessed the registry, so ensured it exists:
Q_ASSERT(calendarRegistry.exists());
return calendarRegistry->registerName(this, name); return calendarRegistry->registerName(this, name);
} }

View File

@ -133,7 +133,7 @@ public:
static QStringList availableCalendars(); static QStringList availableCalendars();
protected: protected:
QCalendarBackend(const QString &name, QCalendar::System id = QCalendar::System::User); QCalendarBackend(const QString &name, QCalendar::System system = QCalendar::System::User);
// Locale support: // Locale support:
virtual const QCalendarLocale *localeMonthIndexData() const = 0; virtual const QCalendarLocale *localeMonthIndexData() const = 0;