Retry registration serialization failures instead of misreporting them as duplicates

CHANGELOG.md

@@ -18,6 +18,7 @@ All notable changes to this project are documented here. This project follows [S
 - Remember-me enabled without a signing key, and `usePersistentTokens=true` without a `PersistentTokenRepository` bean, now log explicit warnings instead of silently skipping/downgrading.
 
 ### Fixed
+- Concurrent registrations of **different** emails could deadlock under the SERIALIZABLE registration transaction (MariaDB error 1213) and the victim was misreported as "user already exists": the person saw the registration-pending page while no account was created and no verification email sent. Serialization failures are now retried in a fresh transaction (up to 3 attempts) — a genuine same-email race still returns the 409/anti-enumeration response, and exhausted retries surface as an error instead of a false success. Affects all prior versions; found via the demo app's concurrent Playwright suite.
 - `user.security.rememberMe.usePersistentTokens` was only honored in its exact camelCase spelling; the kebab-case spelling advertised by the generated configuration metadata (`user.security.remember-me.use-persistent-tokens`) bound the properties bean but never created the persistent-token repository, silently downgrading remember-me to hash-based tokens (which cannot be revoked server-side). The condition now accepts every relaxed spelling.
 
 ## [5.2.0] - 2026-08-12

src/main/java/com/digitalsanctuary/spring/user/service/UserService.java

@@ -208,6 +208,12 @@
 	/** The user role name. */
 	private static final String USER_ROLE_NAME = "ROLE_USER";
 
+	/** Attempts for the SERIALIZABLE registration write before a serialization failure is surfaced. */
+	private static final int REGISTRATION_SERIALIZATION_ATTEMPTS = 5;
+
+	/** Base delay between registration serialization retries; the actual delay grows per attempt and is jittered. */
+	private static final long REGISTRATION_RETRY_BASE_DELAY_MS = 25;
+
 	/** The user repository. */
 	private final UserRepository userRepository;
 
@@ -288,13 +294,15 @@
 	 * @param newUserDto the data transfer object containing the user registration
 	 *                   information
 	 * <p>
-	 * Runs with {@link Isolation#SERIALIZABLE} isolation to close the duplicate-registration
-	 * race when two requests register the same email concurrently. The {@link #emailExists}
-	 * pre-check handles the common case, but a concurrent insert can still fail at commit; in
-	 * that case the resulting {@link DataIntegrityViolationException} (unique-constraint
-	 * violation) or serialization failure ({@link CannotAcquireLockException} /
-	 * {@link ConcurrencyFailureException}) is translated into a {@link UserAlreadyExistException}
-	 * (HTTP 409) rather than surfacing as a 500. Unrelated failures are never swallowed.
+	 * The DB write runs with {@link Isolation#SERIALIZABLE} isolation to close the
+	 * duplicate-registration race when two requests register the same email concurrently: a losing
+	 * duplicate insert ({@link DataIntegrityViolationException}) is translated into a
+	 * {@link UserAlreadyExistException} (HTTP 409). A serialization failure
+	 * ({@link CannotAcquireLockException} / {@link ConcurrencyFailureException}) — which can also be
+	 * caused by a concurrent registration of a <em>different</em> email deadlocking on index gap locks
+	 * — is retried in a fresh transaction (see {@link #persistWithSerializationRetry}); exhausted
+	 * retries propagate the failure rather than misreporting it as an existing account. Unrelated
+	 * failures are never swallowed.
 	 * </p>
 	 *
 	 * @implNote This method is {@link Propagation#NOT_SUPPORTED}: the slow bcrypt hash runs with no
@@ -346,12 +354,80 @@
 
 		// Persist through the proxy so the SERIALIZABLE transaction actually applies (a direct
 		// this.persistNewUserAccount(...) self-invocation would bypass the proxy and run no transaction).
-		User saved = self.persistNewUserAccount(user);
+		User saved = persistWithSerializationRetry(user);
 		// authWithoutPassword(saved);
 		timeLogger.end();
 		return saved;
 	}
 
+	/**
+	 * Invokes {@link #persistNewUserAccount(User)} through the proxy, retrying when the SERIALIZABLE
+	 * transaction fails to serialize (deadlock / lock-acquisition failure,
+	 * {@link ConcurrencyFailureException}).
+	 *
+	 * <p>
+	 * A serialization failure does NOT imply a duplicate: two concurrent registrations of
+	 * <em>different</em> emails can deadlock on index gap locks under SERIALIZABLE isolation. Each
+	 * retry runs a fresh transaction whose {@code emailExists} pre-check distinguishes the two cases —
+	 * a genuine same-email race throws {@link UserAlreadyExistException} (HTTP 409), while a
+	 * different-email deadlock simply succeeds on retry. When every attempt fails to serialize, the
+	 * last {@link ConcurrencyFailureException} propagates so the failure is visible to the caller
+	 * (HTTP 500) instead of being misreported as an existing account while no account was created.
+	 * </p>
+	 *
+	 * @param user the fully built user entity (password already encoded)
+	 * @return the saved user entity
+	 * @throws UserAlreadyExistException if an account with the same email already exists
+	 * @throws ConcurrencyFailureException if every attempt fails to serialize
+	 */
+	private User persistWithSerializationRetry(final User prototype) {
+		ConcurrencyFailureException lastFailure = null;
+		for (int attempt = 1; attempt <= REGISTRATION_SERIALIZATION_ATTEMPTS; attempt++) {
+			try {
+				// Each attempt persists a FRESH entity: a rolled-back attempt can leave the passed instance
+				// carrying persistence state (a generated id, Hibernate-managed collections such as
+				// passwordHistoryEntries), and re-saving that instance fails with optimistic-locking or
+				// orphan-delete errors instead of performing a clean INSERT.
+				return self.persistNewUserAccount(copyForInsert(prototype));
+			} catch (ConcurrencyFailureException e) {
+				lastFailure = e;
+				log.warn("UserService.persistWithSerializationRetry: serialization failure on attempt {}/{} for email {}: {}",
+						attempt, REGISTRATION_SERIALIZATION_ATTEMPTS, prototype.getEmail(), e.getClass().getSimpleName());
+				if (attempt < REGISTRATION_SERIALIZATION_ATTEMPTS) {
+					try {
+						// Growing, jittered delay: concurrent losers retrying in lockstep would keep
+						// deadlocking against each other; jitter desynchronizes them.
+						long delay = REGISTRATION_RETRY_BASE_DELAY_MS * attempt
+								+ java.util.concurrent.ThreadLocalRandom.current().nextLong(REGISTRATION_RETRY_BASE_DELAY_MS * attempt);
+						Thread.sleep(delay);
+					} catch (InterruptedException interrupted) {
+						Thread.currentThread().interrupt();
+						throw lastFailure;
+					}
+				}
+			}
+		}
+		throw lastFailure;
+	}
+
+	/**
+	 * Copies the registration-relevant fields onto a new transient {@link User} for a persist attempt.
+	 * Only the fields set by {@link #registerNewUserAccount(UserDto)} are copied; everything else keeps
+	 * its entity default, exactly as on a first attempt.
+	 *
+	 * @param prototype the user carrying the registration data
+	 * @return a fresh transient copy safe to persist
+	 */
+	private static User copyForInsert(final User prototype) {
+		User user = new User();
+		user.setFirstName(prototype.getFirstName());
+		user.setLastName(prototype.getLastName());
+		user.setPassword(prototype.getPassword());
+		user.setEmail(prototype.getEmail());
+		user.setEnabled(prototype.isEnabled());
+		return user;
+	}
+
 	/**
 	 * Persists a new user account inside a short, serializable transaction.
 	 *
@@ -361,10 +437,11 @@
 	 * connection-holding transaction is open. It runs with {@link Isolation#SERIALIZABLE} to close the
 	 * duplicate-registration race when two requests register the same email concurrently. The
 	 * {@link #emailExists} pre-check handles the common case, but a concurrent insert can still fail at
-	 * commit; in that case the resulting {@link DataIntegrityViolationException} (unique-constraint
-	 * violation) or serialization failure ({@link CannotAcquireLockException} /
-	 * {@link ConcurrencyFailureException}) is translated into a {@link UserAlreadyExistException}
-	 * (HTTP 409) rather than surfacing as a 500. Unrelated failures are never swallowed.
+	 * commit: a unique-constraint violation ({@link DataIntegrityViolationException}) is translated
+	 * into a {@link UserAlreadyExistException} (HTTP 409), while a serialization failure
+	 * ({@link CannotAcquireLockException} / {@link ConcurrencyFailureException}) propagates unchanged
+	 * so the caller's retry ({@link #persistWithSerializationRetry}) can distinguish a same-email race
+	 * from a different-email deadlock. Unrelated failures are never swallowed.
 	 * </p>
 	 *
 	 * <p>
@@ -397,13 +474,14 @@
 			User saved = userRepository.save(user);
 			savePasswordHistory(saved, saved.getPassword());
 			return saved;
-		} catch (DataIntegrityViolationException | ConcurrencyFailureException e) {
-			// A concurrent registration won the race: the unique-email constraint was violated
-			// (DataIntegrityViolationException) or the SERIALIZABLE transaction could not be
-			// serialized (ConcurrencyFailureException, e.g. CannotAcquireLockException). Translate
-			// to a 409 instead of letting it surface as a 500. Only these duplicate/serialization
-			// cases are translated; unrelated exceptions propagate unchanged.
-			log.debug("UserService.persistNewUserAccount: concurrent registration detected for email {}: {}",
+		} catch (DataIntegrityViolationException e) {
+			// A concurrent registration of the SAME email won the race: the unique-email constraint
+			// was violated. Translate to a 409 instead of letting it surface as a 500. A
+			// ConcurrencyFailureException (deadlock / serialization failure) is deliberately NOT
+			// translated here: it can be caused by a concurrent registration of a DIFFERENT email,
+			// so it propagates to the retry in persistWithSerializationRetry, whose fresh-transaction
+			// pre-check distinguishes the two cases. Unrelated exceptions propagate unchanged.
+			log.debug("UserService.persistNewUserAccount: concurrent duplicate registration detected for email {}: {}",
 					user.getEmail(), e.getClass().getSimpleName());
 			throw new UserAlreadyExistException(
 					"There is an account with that email address: " + user.getEmail());

src/test/java/com/digitalsanctuary/spring/user/api/UserApiTest.java

@@ -190,14 +190,32 @@ void tearDown() {
      */
     private void deleteTestUser(String email) {
         // This test is not @Transactional, so cleanup must run in its own committed transaction.
-        txTemplate.executeWithoutResult(status -> {
-            User user = userRepository.findByEmail(email);
-            if (user != null) {
-                passwordResetTokenRepository.deleteByUser(user);
-                verificationTokenRepository.deleteByUser(user);
-                userRepository.delete(user);
+        // Retried because the @Async RegistrationListener can commit a verification token between
+        // deleteByUser and the user delete, failing the FK constraint; the retry's fresh
+        // transaction sees and deletes the late token.
+        for (int attempt = 1;; attempt++) {
+            try {
+                txTemplate.executeWithoutResult(status -> {
+                    User user = userRepository.findByEmail(email);
+                    if (user != null) {
+                        passwordResetTokenRepository.deleteByUser(user);
+                        verificationTokenRepository.deleteByUser(user);
+                        userRepository.delete(user);
+                    }
+                });
+                return;
+            } catch (org.springframework.dao.DataIntegrityViolationException e) {
+                if (attempt >= 3) {
+                    throw e;
+                }
+                try {
+                    Thread.sleep(100);
+                } catch (InterruptedException interrupted) {
+                    Thread.currentThread().interrupt();
+                    throw e;
+                }
             }
-        });
+        }
     }
 
     private String json(Object value) {

src/test/java/com/digitalsanctuary/spring/user/captcha/CaptchaProtectionIntegrationTest.java

@@ -220,16 +220,36 @@ private void drainMailExecutor() {
      * Hard-deletes the test user and any associated tokens (tokens first, FK order). This test is
      * not @Transactional, so cleanup runs in its own committed transaction — same pattern as
      * UserApiTest.
+     *
+     * <p>Retried because the token insert happens on the {@code @Async} RegistrationListener's
+     * executor (not {@code dsMailExecutor}, which {@code drainMailExecutor()} waits on), so a
+     * verification token can commit between {@code deleteByUser} and the user delete and fail the
+     * FK constraint; the retry's fresh transaction sees and deletes it.</p>
      */
     private void deleteTestUser(String email) {
-        txTemplate.executeWithoutResult(status -> {
-            User user = userRepository.findByEmail(email);
-            if (user != null) {
-                passwordResetTokenRepository.deleteByUser(user);
-                verificationTokenRepository.deleteByUser(user);
-                userRepository.delete(user);
+        for (int attempt = 1;; attempt++) {
+            try {
+                txTemplate.executeWithoutResult(status -> {
+                    User user = userRepository.findByEmail(email);
+                    if (user != null) {
+                        passwordResetTokenRepository.deleteByUser(user);
+                        verificationTokenRepository.deleteByUser(user);
+                        userRepository.delete(user);
+                    }
+                });
+                return;
+            } catch (org.springframework.dao.DataIntegrityViolationException e) {
+                if (attempt >= 3) {
+                    throw e;
+                }
+                try {
+                    Thread.sleep(100);
+                } catch (InterruptedException interrupted) {
+                    Thread.currentThread().interrupt();
+                    throw e;
+                }
             }
-        });
+        }
     }
 
     /**

src/test/java/com/digitalsanctuary/spring/user/service/AbstractConcurrentRegistrationTest.java

@@ -25,8 +25,11 @@
 
 /**
  * Validates that the SERIALIZABLE duplicate-registration race protection (UserService.registerNewUserAccount ->
- * persistNewUserAccount, isolation = SERIALIZABLE, with DataIntegrityViolationException / ConcurrencyFailureException
- * translated to UserAlreadyExistException) actually holds on a real, production-grade database — not just on H2.
+ * persistNewUserAccount, isolation = SERIALIZABLE, with DataIntegrityViolationException translated to
+ * UserAlreadyExistException and serialization failures retried in a fresh transaction) actually holds on a real,
+ * production-grade database — not just on H2. Also validates the inverse: concurrent registrations of DIFFERENT
+ * emails, which can deadlock on index gap locks under SERIALIZABLE, must all succeed via the retry rather than
+ * being misreported as duplicates.
  *
  * <p>
  * Two threads race to register the SAME email at the same instant (released together via a CountDownLatch). On a real
@@ -131,6 +134,53 @@ void shouldSerializeConcurrentDuplicateRegistrationWhenTwoThreadsRaceSameEmail()
 		}
 	}
 
+	@RepeatedTest(value = 3, name = "{displayName} [run {currentRepetition}/{totalRepetitions}]")
+	@DisplayName("should register every user when threads race with different emails")
+	void shouldRegisterEveryUserWhenThreadsRaceDifferentEmails() throws InterruptedException {
+		// Distinct emails cannot conflict logically, but their SERIALIZABLE transactions can still deadlock on
+		// index gap locks. Before the serialization retry existed, the deadlock victim was misreported as
+		// UserAlreadyExistException — a silently lost registration behind the anti-enumeration success page.
+		final int threadCount = 6;
+		final CountDownLatch readyLatch = new CountDownLatch(threadCount);
+		final CountDownLatch startLatch = new CountDownLatch(1);
+		final ExecutorService executor = Executors.newFixedThreadPool(threadCount);
+
+		try {
+			final List<String> emails = new ArrayList<>();
+			final List<Future<RegistrationOutcome>> futures = new ArrayList<>();
+			for (int i = 0; i < threadCount; i++) {
+				final String email = "distinct-" + i + "-" + System.nanoTime() + "@test.com";
+				emails.add(email);
+				futures.add(executor.submit(registrationTask(email, readyLatch, startLatch)));
+			}
+
+			assertThat(readyLatch.await(30, TimeUnit.SECONDS))
+					.as("all registration threads should reach the start gate")
+					.isTrue();
+			startLatch.countDown();
+
+			final List<Throwable> failures = new ArrayList<>();
+			for (Future<RegistrationOutcome> future : futures) {
+				final RegistrationOutcome outcome = collect(future);
+				if (outcome.user == null) {
+					failures.add(outcome.error);
+				}
+			}
+
+			assertThat(failures)
+					.as("every distinct-email registration must succeed — a deadlock between them must be retried, "
+							+ "never surfaced (and never misreported as UserAlreadyExistException)")
+					.isEmpty();
+			for (String email : emails) {
+				assertThat(userRepository.findByEmail(email.toLowerCase()))
+						.as("user row should exist for %s", email)
+						.isNotNull();
+			}
+		} finally {
+			executor.shutdownNow();
+		}
+	}
+
 	private Callable<RegistrationOutcome> registrationTask(final String email, final CountDownLatch readyLatch,
 			final CountDownLatch startLatch) {
 		return () -> {