Skip to content

Remove global digestLock serialization in digest password storage schemes - #667

Merged
vharseko merged 6 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-2
Aug 6, 2026
Merged

Remove global digestLock serialization in digest password storage schemes#667
vharseko merged 6 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-2

Conversation

@vharseko

@vharseko vharseko commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

Removes another global serialization point on the BIND hot path, found while
profiling after #660: every digest-based password storage scheme shares a
single MessageDigest instance guarded by synchronized (digestLock).
Schemes are server-wide singletons, so all concurrent bind password
verifications for a given scheme (e.g. every {SSHA} bind) serialize on one
monitor — with the CPU-bound hashing executed under the lock. The same
pattern is removed from the CRAM-MD5 SASL handler and from the UNIX
crypt(3) implementation behind the {CRYPT} scheme.

Problem

SaltedSHA1PasswordStorageScheme.passwordMatches() (and its siblings) hash
the candidate password inside a global synchronized block:

synchronized (digestLock)
{
  userDigestBytes = messageDigest.digest(plainPlusSalt);
  ...
}

Under concurrent BIND load (packaged server, je backend, 5000 users with
{SSHA} passwords, 200 client connections re-binding as random users via
authrate), JFR jdk.JavaMonitorEnter shows this is the dominant monitor
contention once the access-log queues are out of the way: 949 blocking
events totalling ~19 s per 80 s recording across 16 worker threads
, with
individual waits of 10–18 ms. After this change the monitor disappears from
the profile entirely.

Change

  • Digest schemes + CRAM-MD5 (Salted{SHA1,SHA256,SHA384,SHA512,MD5},
    SHA1, MD5, CRAMMD5SASLMechanismHandler): replace the shared
    MessageDigest + digestLock with a ThreadLocal<MessageDigest>
    (ThreadLocal.withInitial); the synchronized wrappers are removed,
    bodies are otherwise unchanged (including the Arrays.fill password
    scrubbing in finally). initialize* still performs a fail-fast
    MessageDigest.getInstance() so an unavailable algorithm keeps throwing
    InitializationException at startup, as before.
  • util/Crypt.java (UNIX crypt(3)/DES used by the {CRYPT} scheme's
    unix mode): the shared SubCrypt working-buffer state guarded by the same
    lock becomes a ThreadLocal<SubCrypt>. This also fixes a pre-existing
    race: _crypt() returns a reference to the shared _iobuf buffer, and
    crypt() converted it to bytes after releasing the lock, so a
    concurrent call could overwrite the result while it was being read.

Testing

  • mvn -P precommit -pl opendj-server-legacy verify for all affected
    scheme test cases + CRAMMD5SASLMechanismHandlerTestCase +
    CryptPasswordStorageSchemeTestCase: 357 tests, 0 failures.
  • Concurrent bind benchmark (same harness as Remove per-bind global lock contention in AuthenticatedUsers #660) confirms the digestLock
    monitor is gone from JFR/jstack profiles; on an 8-core machine the
    operating point is CPU-saturated so throughput is unchanged (+~1%), the
    win is removal of the serialization ceiling on wider machines.

Files

  • opendj-server-legacy/src/main/java/org/opends/server/extensions/Salted{SHA1,SHA256,SHA384,SHA512,MD5}PasswordStorageScheme.java
  • opendj-server-legacy/src/main/java/org/opends/server/extensions/{SHA1,MD5}PasswordStorageScheme.java
  • opendj-server-legacy/src/main/java/org/opends/server/extensions/CRAMMD5SASLMechanismHandler.java
  • opendj-server-legacy/src/main/java/org/opends/server/util/Crypt.java

vharseko added 2 commits July 2, 2026 18:00
…emes

Every digest-based password storage scheme (and the CRAM-MD5 SASL handler)
shared a single MessageDigest instance guarded by synchronized(digestLock).
Schemes are server-wide singletons, so all concurrent bind password
verifications for a given scheme were serialized on one monitor, with the
CPU-bound hashing executed under the lock (JFR: ~19s of blocking per 80s
window at 16 worker threads under concurrent {SSHA} bind load).

Replace the shared instance with ThreadLocal<MessageDigest>. Initialization
still fails fast with InitializationException if the algorithm is
unavailable. Password scrubbing (Arrays.fill in finally) is preserved.
Crypt (the unix crypt(3)/DES implementation behind the {CRYPT} storage
scheme's unix mode) kept all of its mutable working buffers in a single
shared SubCrypt instance guarded by synchronized(digestLock). The scheme is
a server-wide singleton, so all concurrent {CRYPT} password encodings and
verifications serialized on one monitor, with the DES work executed under
the lock.

Replace the shared SubCrypt with a ThreadLocal<SubCrypt>. This also fixes a
pre-existing race: _crypt() returns a reference to the shared _iobuf buffer,
and crypt() converted it to bytes after releasing the lock, so a concurrent
call could overwrite the result while it was being read.
@vharseko
vharseko merged commit 1bda529 into OpenIdentityPlatform:master Aug 6, 2026
14 checks passed
@vharseko
vharseko deleted the features/bind-2 branch August 6, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement performance Performance / concurrency / lock-contention work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants