Remove the pre-update lock on a user's attribute rows

core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCRealmConstants.java

@@ -490,6 +490,10 @@ public final class JDBCRealmConstants {
     public static final String UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_SQL = "UPDATE UM_USER_ATTRIBUTE SET UM_ATTR_VALUE=? " +
             "WHERE UM_ATTR_NAME=? AND UM_PROFILE_ID=? AND UM_USER_ID=(SELECT UM_ID FROM UM_USER WHERE UM_USER_ID=? AND " +
             "UM_TENANT_ID=?) AND UM_TENANT_ID=?";
+    public static final String UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL_SQL = "UPDATE UM_USER_ATTRIBUTE SET " +
+            "UM_ATTR_VALUE=? WHERE UM_ATTR_NAME=CAST(? AS VARCHAR(1000)) AND UM_PROFILE_ID=CAST(? AS VARCHAR(1000)) " +
+            "AND UM_USER_ID=(SELECT UM_ID FROM UM_USER WHERE UM_USER_ID=CAST(? AS VARCHAR(1000)) AND " +
+            "UM_TENANT_ID=?) AND UM_TENANT_ID=?";
     public static final String SELECT_USER_PROPERTIES_WITH_ID_SQL = "SELECT UM_ID FROM UM_USER_ATTRIBUTE WITH " +
             "(UPDLOCK) WHERE UM_USER_ID=(SELECT UM_ID FROM UM_USER WHERE UM_USER_ID=? AND UM_TENANT_ID=?) " +
             "ORDER BY UM_ID;";
@@ -612,6 +616,8 @@ public static final class TX_ISOLATION_LEVELS {
     public static final String ADD_USER_TO_ROLE_MSSQL = "AddUserToRoleSQL-mssql";
     public static final String ADD_ROLE_TO_USER_MSSQL = "AddRoleToUserSQL-mssql";
     public static final String ADD_USER_PROPERTY_MSSQL = "AddUserPropertySQL-mssql";
+    public static final String UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL =
+            "UpdateUserPropertyWithIDOptimizedSQL-mssql";
     //openedge
     public static final String ADD_USER_TO_ROLE_OPENEDGE_SQL = "INSERT INTO UM_USER_ROLE (UM_USER_ID, UM_ROLE_ID, UM_TENANT_ID) SELECT UU.UM_ID, UR.UM_ID, ? FROM UM_USER UU, UM_ROLE UR WHERE UU.UM_USER_NAME=? AND UU.UM_TENANT_ID=? AND UR.UM_ROLE_NAME=? AND UR.UM_TENANT_ID=?";
     public static final String ADD_ROLE_TO_USER_OPENEDGE_SQL = "INSERT INTO UM_USER_ROLE (UM_ROLE_ID, UM_USER_ID, UM_TENANT_ID) SELECT UR.UM_ID, UU.UM_ID, ? FROM UM_ROLE UR, UM_USER UU WHERE UR.UM_ROLE_NAME=? AND UR.UM_TENANT_ID=? AND UU.UM_USER_NAME=? AND UU.UM_TENANT_ID=?";

core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/UniqueIDJDBCUserStoreManager.java

@@ -92,6 +92,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.TimeZone;
+import java.util.TreeMap;
 import java.util.stream.Collectors;
 
 import javax.sql.DataSource;
@@ -3238,6 +3239,20 @@ private List<String> chunkExtendedAttributeValues(String value, int maxLength, i
         return valueChunks;
     }
 
+    /**
+     * Orders the claims of a batch update deterministically.
+     * <p>
+     * Two concurrent updates that share a claim would otherwise be free to take the same row locks in opposite
+     * orders and deadlock, because the order is whatever the caller's map happens to iterate in.
+     *
+     * @param properties Claim URI to value, in the caller's order.
+     * @return The same entries, ordered by claim URI. Package private for testing.
+     */
+    static Set<Map.Entry<String, String>> orderClaimsForBatch(Map<String, String> properties) {
+
+        return new TreeMap<>(properties).entrySet();
+    }
+
     /**
      * Update properties as a batch.
      *
@@ -3267,6 +3282,10 @@ private void updateProperties(Connection dbConnection, String userID, Map<String
 
         String sqlStmt =
                 realmConfig.getUserStoreProperty(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID + "-" + type);
+        if (sqlStmt == null) {
+            sqlStmt = realmConfig
+                    .getUserStoreProperty(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED + "-" + type);
+        }
         if (sqlStmt == null) {
             sqlStmt = realmConfig.getUserStoreProperty(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED);
         }
@@ -3285,7 +3304,7 @@ private void updateProperties(Connection dbConnection, String userID, Map<String
             boolean useNString = shouldUseNString(dbConnection);
             prepStmt = dbConnection.prepareStatement(sqlStmt);
 
-            for (Map.Entry<String, String> entry : properties.entrySet()) {
+            for (Map.Entry<String, String> entry : orderClaimsForBatch(properties)) {
                 String propertyName = entry.getKey();
                 String propertyValue = entry.getValue();
                 if (sqlStmt.contains(UserCoreConstants.UM_TENANT_COLUMN)) {
@@ -3302,14 +3321,6 @@ private void updateProperties(Connection dbConnection, String userID, Map<String
                 }
             }
 
-            /*
-             Lock all the user's attribute rows in one statement before writing any of them. Otherwise concurrent
-             batch updates of the same user can each hold a lock the other one still needs, and deadlock. The lock
-             hint is SQL Server syntax, and only SQL Server has been observed to deadlock here.
-            */
-            if (MSSQL.equalsIgnoreCase(type)) {
-                selectRowsForUpdate(dbConnection, userID);
-            }
             int[] counts = prepStmt.executeBatch();
             if (log.isDebugEnabled()) {
                 int totalUpdated = 0;
@@ -5053,45 +5064,6 @@ public boolean isUniqueUserIdEnabled() {
         return true;
     }
 
-    /**
-     * Select and update lock the user attribute rows before the update operation.
-     *
-     * @param dbConnection Database connection.
-     * @param userID       User id of the user.
-     * @throws UserStoreException If an error occurred while executing statement.
-     */
-    private void selectRowsForUpdate(Connection dbConnection, String userID) throws UserStoreException {
-
-        String sqlStmt = realmConfig.getUserStoreProperty(JDBCRealmConstants.SELECT_USER_PROPERTIES_WITH_ID_OPTIMIZED);
-        try (PreparedStatement prepStmt = dbConnection.prepareStatement(sqlStmt)) {
-            prepStmt.setString(1, userID);
-            prepStmt.setInt(2, tenantId);
-            prepStmt.setInt(3, tenantId);
-            prepStmt.executeQuery();
-        } catch (SQLException e) {
-            String errorMessage = "Error while selecting rows for updating user attributes";
-            if (log.isDebugEnabled()) {
-                log.debug(errorMessage, e);
-            }
-            throw new UserStoreException(errorMessage, e);
-        }
-    }
-
-    /**
-     * Check if the DB is MSSQL.
-     *
-     * @return true if MSSQL, false otherwise.
-     * @throws UserStoreException if error occurred while getting database type.
-     */
-    private boolean isMSSQLDB(Connection dbConnection) throws UserStoreException {
-
-        try {
-            return MSSQL.equalsIgnoreCase(DatabaseCreator.getDatabaseType(dbConnection));
-        } catch (Exception e) {
-            throw new UserStoreException("Error while retrieving the DB type. ", e);
-        }
-    }
-
     /**
      * Paginate a group list.
      *

core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/util/JDBCRealmUtil.java

@@ -489,6 +489,11 @@ public static Map<String, String> getSQL(Map<String, String> properties) {
             properties.put(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID,
                     JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_SQL);
         }
+        if (!properties.containsKey(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED)
+                && !properties.containsKey(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL)) {
+            properties.put(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL,
+                    JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL_SQL);
+        }
         if (!properties.containsKey(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED)) {
             properties.put(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED,
                     JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_SQL);

core/org.wso2.carbon.user.core/src/test/java/org/wso2/carbon/user/core/jdbc/ClaimUpdatePredicateSQLTest.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com).
+ *
+ * WSO2 LLC. licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.wso2.carbon.user.core.jdbc;
+
+import org.testng.annotations.Test;
+import org.wso2.carbon.user.core.util.JDBCRealmUtil;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+/**
+ * Covers the SQL Server claim update path, where a regression is silent: nothing throws and nothing
+ * logs, the statement just stops seeking and starts locking the user's whole attribute set again.
+ */
+public class ClaimUpdatePredicateSQLTest {
+
+    @Test
+    public void testMSSQLStatementCastsOnlyTheVarcharPredicates() {
+
+        String sql = JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL_SQL;
+        for (String column : new String[] { "UM_ATTR_NAME", "UM_PROFILE_ID", "UM_USER_ID" }) {
+            assertTrue(sql.contains(column + "=CAST(? AS VARCHAR"),
+                    column + " is a VARCHAR column and must be compared against a VARCHAR parameter, otherwise the "
+                            + "conversion lands on the column and the predicate cannot seek: " + sql);
+        }
+        assertTrue(sql.startsWith("UPDATE UM_USER_ATTRIBUTE SET UM_ATTR_VALUE=? "),
+                "UM_ATTR_VALUE is NVARCHAR and is bound as Unicode where the user store asks for it, so casting it "
+                        + "would silently mangle non-ASCII claim values: " + sql);
+    }
+
+    @Test
+    public void testMSSQLStatementKeepsTheGenericParameterOrder() {
+
+        String generic = JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_SQL;
+        String mssql = JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL_SQL;
+        assertEquals(mssql.replaceAll("CAST\\(\\? AS VARCHAR\\(\\d+\\)\\)", "?").replaceAll("\\s+", " ").trim(),
+                generic.replaceAll("\\s+", " ").trim(),
+                "Both statements are bound by the same code, in a fixed parameter order, so the SQL Server variant "
+                        + "must differ from the generic one only by the casts.");
+    }
+
+    @Test
+    public void testMSSQLStatementIsRegisteredUnderTheKeyTheLookupUses() {
+
+        assertEquals(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL,
+                JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED + "-mssql",
+                "updateProperties looks the statement up as the property name plus the database type, so the two "
+                        + "must agree - if they drift, the cast is never read and nothing reports it.");
+        assertEquals(JDBCRealmUtil.getSQL(new HashMap<>())
+                        .get(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL),
+                JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL_SQL,
+                "A user store that overrides nothing must pick up the SQL Server variant.");
+        assertFalse(JDBCRealmUtil.getSQL(new HashMap<>(Map.of(
+                        JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED, "UPDATE the store's own SQL")))
+                        .containsKey(JDBCRealmConstants.UPDATE_USER_PROPERTY_WITH_ID_OPTIMIZED_MSSQL),
+                "The SQL Server variant is looked up first, so it must not be added to a user store that configured "
+                        + "its own generic statement - that would override what was configured.");
+    }
+
+    @Test
+    public void testTheBatchIsOrderedIndependentlyOfTheCallersMap() {
+
+        Map<String, String> properties = new LinkedHashMap<>();
+        properties.put("http://wso2.org/claims/mobile", "1");
+        properties.put("http://wso2.org/claims/emailaddress", "2");
+        properties.put("http://wso2.org/claims/country", "3");
+
+        Map<String, String> reversed = new LinkedHashMap<>();
+        properties.entrySet().stream().sorted((a, b) -> b.getKey().compareTo(a.getKey()))
+                .forEach(e -> reversed.put(e.getKey(), e.getValue()));
+
+        assertEquals(claimOrder(properties), claimOrder(reversed),
+                "Two callers passing the same claims in different orders must produce the same batch order, "
+                        + "otherwise they can take the same row locks in opposite orders and deadlock.");
+        assertEquals(claimOrder(properties), Arrays.asList("http://wso2.org/claims/country",
+                "http://wso2.org/claims/emailaddress", "http://wso2.org/claims/mobile"));
+    }
+
+    private List<String> claimOrder(Map<String, String> properties) {
+
+        return UniqueIDJDBCUserStoreManager.orderClaimsForBatch(properties).stream()
+                .map(Map.Entry::getKey).collect(Collectors.toList());
+    }
+}

distribution/kernel/carbon-home/dbscripts/mssql.sql

@@ -414,6 +414,10 @@ DROP INDEX UM_USER_ATTRIBUTE.UM_USER_ID_INDEX
 CREATE INDEX UM_USER_ID_INDEX ON UM_USER_ATTRIBUTE(UM_USER_ID);
 
 CREATE INDEX UM_ATTR_NAME_VALUE_INDEX ON UM_USER_ATTRIBUTE(UM_ATTR_NAME, UM_ATTR_VALUE);
+IF NOT EXISTS (SELECT 1 FROM SYS.INDEXES WHERE NAME = 'UM_ATTR_USER_ID_TENANT_ID_NAME_PROFILE_INDEX'
+               AND OBJECT_ID = OBJECT_ID(N'[dbo].[UM_USER_ATTRIBUTE]'))
+CREATE INDEX UM_ATTR_USER_ID_TENANT_ID_NAME_PROFILE_INDEX ON UM_USER_ATTRIBUTE(UM_USER_ID,
+             UM_TENANT_ID, UM_ATTR_NAME, UM_PROFILE_ID) INCLUDE (UM_ATTR_VALUE);
 
 --CREATE TABLE   UM_ROLE