Open Legioth opened 3 years ago
The same approach for synchronization of internal state could also be used by classes like PresenceManager
(aka PresenceAdapter
) that are centered around a ConnectionContext
instance (once we get rid of setTopic
).
The design of classes like
TopicConnection
andCollaborationMap
is implicitly assuming that usage is from a thread that holds the same session lock that is also used by the associatedComponentConnectionContext
. In this way, there are never any synchronization issues between actions triggered from application code and actions triggered from collaborative changes (which are delegated troughConnectionContext::dispatchAction)
. There's also a per-topic lock that helps protect the integrity of the data in the topic itself, but this lock does not always provide clearly ordered operations from the application's point of view.State in
TopicConnection
does currently have these characteristics:closeRegistration
is assigned in the activation handler which is synchronized by the connection context (assuming it's implementing synchronization). The field is read and assigned asnull
incloseWithoutDeactivating
which is invoked from the activation handler, fromTopicConnectionRegistration::remove
which is called from application code, and from exception handlers which may be called when processing data changes while holding the topic lock.deactivateRegistrations
is modified from application code when adding subscribers to list or maps (from application code) and from the activation handler. Contents are read and cleared indeactivate()
which can be run from the activation handler, from application code, and through error handlers from topic changes holding the topic lock.subscribersPerMap
is modified fromsubscribe
which holds the topic lock and fromunsubscribeFromMap
which is called directly through application code. The collection is iterated (and in error cases potentially modified) inhandleMapChange
which is called for change events while holding the topic lock. The same pattern is repeated forsubscribersPerList
.active
is set from the activation handler and reset fromcloseWithoutDeactivating
which can be called from all the places. The field is read without any special synchronization fromensureActiveConnection()
which is triggered for all operations from application code.Taken as a whole, all mutable state in
TopicConnection
can be concurrently accessed from three different directions: application code (which may be assumed to hold the session lock), activation handler (which holds the session lock when usingComponentConnectionContext
), and topic events (which hold the topic lock but does typically not hold the (right) session lock). 💥It seems like updating
handleMapChange
andhandleListChange
to dispatch through the connection context would address all the cases where state is accessed from the topic side, thus only leaving the paths through application code and through the activation handler.We could ensure application code invocations are always synchronized with regards to the activation handler invocations if we would introduce a method in
ConnectionContext
that would assert synchronization is present (e.g. by checking that aReentrantLock
is held by the current thread) and triggering that check through all application code entry points.