Verifies the integrety of the projects, as builds are executed immediately after a code change was detected. This plan provides no artifiacts (use a nightly build instead).

Build: #2582 was successful Changes by Guus der Kinderen

Build result summary

Details

Completed
Queue duration
< 1 second
Duration
11 minutes
Labels
None
Agent
ip-172-31-27-151.eu-central-1.compute.internal
Revision
30ab578e48227a8b2b7de165f690de980a8f528b
Total tests
1398
Successful since
#2581 ()

Tests

Code commits

Author Commit Message Commit date
Guus der Kinderen Guus der Kinderen 1e109775535b8c8c97d51843501cbec2ee8e95f9 Fixed a typo
Guus der Kinderen Guus der Kinderen 30ab578e48227a8b2b7de165f690de980a8f528b OF-2824: Relax introduced fail-fast behavior
In commit ef582e131f41bcb61bfb75c083986b92b0bcd002 fail-fast behavior was introduced that causes exceptions to be thrown when certain methods are invoked using a bare JID while the expected argument is a full JID.

For certain lookups, these unexpected bare JIDs appear to be used all the time, causing Openfire to become pretty unusable.

In this commit, the invocations that do not _modify_ cache content are short-circuited when a bare JID argument is used. This has the benefit of preventing the acquisition of a cluster-wide lock.
Guus der Kinderen Guus der Kinderen 67108eddd2346099eab7cd4081286548c8bf3d70 OF-2824: Manipulation of client-related caches in RoutingTable using the same lock
RoutingTableImpl has three closely related caches, that are used to represent the state of client session routes:
- `usersSessionsCache`
- `anonymousUsersCache`
- `usersCache`

Each value in the first cache is expected to correspond to a value in one of the other two caches.

Under OF-2824, a bug is described where `usersCache` contains values that are _not_ in `usersSessionsCache`. That shouldn't be possible.

Prior to this commit, manipulation of these caches is performed under a lock obtained from each of the caches. This means that the overall operation of adding an entry to `usersSessionsCache` and one of the other two caches is _not_ guarded by one singular lock (instead, two locks are used, each guarding the operation pertaining to that particular cache). This leaves room for a race-condition.

This commit addresses the race condition by using one singular lock to guard manipulations in all of these caches.

As all caches use a JID (in either bare or full form) as their key value, the singular lock introduced by this commit is based on the bare JID of the key that's being manipulated. This lock is obtained from the `usersSessionsCache`.

This change can lead to more lock contention (as more operations are guarded by the same lock). Simultaneously, less acquiring of locks will take place (as many operations previously required two locks to be acquired, while now, only one is needed. What the effects are on performance is as of yet undetermined.

This commit also introduces some related, minor changes:
- Logged messages are made more consistent
- Some operations have been moved outside of the protection of a (potentially cluster-wide) lock, to improve performance
- Where methods expect to be called with a full JID, exceptions are thrown when a bare JID is used. This fail-fast behavior is intended to uncover any existing or future bugs.
Guus der Kinderen Guus der Kinderen 3c7a0c684939ca5738868689360d22cb3583904a OF-2824: Remove cluster-cache optimization
It is suspected that, under race conditions, the optimization removed by this commit introduces data inconsistency.

The optimization prevents modification of a second cache, as through the output of manipulation of the first cache it can be deduced that the second cache should already be in the expected state. By skipping manipulation of that second cache, a cluster-wide operation is prevented, which improves performance.

The presence-based override for the optimization - one that I frankly do not understand - becomes obsolete by this change, and is also removed.

The change introduced by this commit trades performance for more reliable data consistency. As an added benefit, the code becomes less complex, reducing maintenance costs.

This commit changes the public signature of the `addClientRoute` method of `RoutingTable`: it no longer returns a value. The return value is currently not used by Openfire's own code. It was used for only two days, back in 2007: it was introduced in commit a940eeff4f72e4e9da70fcd0b4a1db1b3c40cd8d where it was used to update a statistic. This statistic got removed two days later, in commit 67f9ab65c36b8a38f5ab7c480415897839da21f0. Given the nature of the code (RoutingTable being _very_ low level), it is not expected that third-party code uses this method. It contract change should therefor be reasonably safe to do.
Guus der Kinderen Guus der Kinderen 4570e79b926538fb137f8b1a00dff10b10f57b37 Add code comment explaining re-entrant lock usage

Jira issues

IssueDescriptionStatus
Unknown Issue TypeOF-2824Could not obtain issue details from Jira