Skip to content

Remove global digestLock serialization in digest password storage schemes#667

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-2
Open

Remove global digestLock serialization in digest password storage schemes#667
vharseko wants to merge 2 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.
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.

2 participants