Repository: microsoft/garnet
File: libs/server/ACL/UserHandle.cs (C#)

Code before:
        /// </summary>
        public User user;

        /// <summary>
        /// Constructor for a <see cref="UserHandle"/>.
        /// </summary>
        /// <param name="user">The <see cref="User"/> the handle will reference.</param>
        public UserHandle(User user) => this.user = user;

        /// <summary>
        /// Returns the current version of the <see cref="User"/> with the latest modifications.
        /// </summary>
        /// <returns>Returns the current version of the <see cref="User"/> with the latest modifications.</returns>
        public User User
        {
            get => this.user;
        }

        /// <summary>
        /// Attempts to set the <see cref="User"/> for a handle.
        /// </summary>
        /// <param name="newUser">A <see cref="User"/> that should secede the existing <see cref="User"/> referred to by the handle.</param>
        /// <param name="replacedUser">The <see cref="User"/> expected to be replaced.</param>
        /// <returns>True if the assignment was performed; otherwise false.</returns>
        public bool TrySetUser(User newUser, User replacedUser)
            => Interlocked.CompareExchange(ref this.user, newUser, null) == null ||
                Interlocked.CompareExchange(ref this.user, newUser, replacedUser) == replacedUser;
    }
}

Review comment (suggestion):
The first Interlocked.CompareExchange - if it succeeds - breaks the contract of the method, which seems to say that we atomically test and swap existing user to new user, only if the old image was `replacedUser`.

Code after:
        /// The <see cref="User"/> referred to by the <see cref="UserHandle"/>.
        /// </summary>
        public User user;

        /// <summary>
        /// Constructor for a <see cref="UserHandle"/>.
        /// </summary>
        /// <param name="user">The <see cref="User"/> the handle will reference.</param>
        /// <exception cref="ArgumentNullException">Thrown when <paramref name="user"/> is <c>null</c>.</exception>
        public UserHandle(User user)
        {
            if (user == null)
            {
                throw new ArgumentNullException(nameof(user));
            }

            this.user = user;
        }


        /// <summary>
        /// Returns the current version of the <see cref="User"/> with the latest modifications.
        /// </summary>
        /// <returns>Returns the current version of the <see cref="User"/> with the latest modifications.</returns>
        public User User => user;

        /// <summary>
        /// Attempts to set the <see cref="User"/> for a handle.
        /// </summary>
        /// <param name="newUser">A <see cref="User"/> that should secede the existing <see cref="User"/> referred to by the handle.</param>
        /// <param name="replacedUser">The <see cref="User"/> expected to be replaced.</param>
        /// <returns>True if the assignment was performed; otherwise false.</returns>
        /// <exception cref="ArgumentNullException">Thrown when <paramref name="newUser"/> is <c>null</c>.</exception>
        public bool TrySetUser(User newUser, User replacedUser)
        {
            if (newUser == null)
            {
                throw new ArgumentNullException(nameof(newUser));
            }

            return Interlocked.CompareExchange(ref this.user, newUser, replacedUser) == replacedUser;
        }
    }
}