Skip to content

FileChangelogDB.getOrCreateReplicaDB() races shutdownDB(): a replica DB created during shutdown is never released #813

Description

@vharseko

A replica DB created while the changelog is shutting down is never released: it stays
registered as a monitor provider, and its log is never released either.

The window

FileChangelogDB.getOrCreateReplicaDB() reads the shutdown flag in its loop condition:

// FileChangelogDB.java:193
while (!shutdown.get())
{
  final ConcurrentMap<Integer, FileReplicaDB> domainMap = getExistingOrNewDomainMap(baseDN);
  final Pair<FileReplicaDB, Boolean> result = getExistingOrNewReplicaDB(domainMap, serverId, baseDN, server);

The domain map is inserted afterwards, in getExistingOrNewDomainMap() at :230, and the DB
is created and registered in getExistingOrNewReplicaDB() at :266-281. Meanwhile
shutdownDB() flips the flag with a CAS at :336 and drains domainToReplicaDBs at :359.

A caller which reads false just before that CAS therefore inserts its domain map into a map
which has already been drained, and creates a FileReplicaDB which nothing will ever shut
down. FileReplicaDB registers its monitor provider in its constructor and deregisters it in
shutdown() (FileReplicaDB.java:118-119 and :221), and that shutdown() is only ever
reached from the drain in FileChangelogDB.shutdownDB(). So the monitor provider stays
registered for the lifetime of the process, and the log stays referenced — which is what
msgID 274 (Log.releaseLog(), "must be released but it is not referenced") reports.

Observed on CI as a monitor provider of a replication server which had stopped, still
registered ten seconds later, together with msgID 274 on the replica log of that changelog.

Proposed fix

Read the flag again inside the synchronized (domainMap) block which already guards the
creation:

// FileChangelogDB.java:266
if (domainToReplicaDBs.get(baseDN) != domainMap)
{
  return null;
}
if (shutdown.get())
{ // a shutdown was initiated after the domain map was inserted: it would not be drained
  return null;
}

Reading false under that lock means the CAS in shutdownDB() has not run yet, so its
iterator does not exist yet either, so it will see the domain map — inserted before the lock
was taken — and will have to block on the same monitor to drain it. Reading true returns
null, and the loop in getOrCreateReplicaDB() then throws
ERR_CANNOT_CREATE_REPLICA_DB_BECAUSE_CHANGELOG_DB_SHUTDOWN, which is what a caller racing a
shutdown is meant to get.

There is one insertion site and two removal sites for domainToReplicaDBs, so the case
analysis is closed.

Credit

Diagnosed by @maximthomas while reviewing #805.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions