Skip to content

Commit

Permalink
agent: fix acceptContact inconsistent state after app restart (#1412)
Browse files Browse the repository at this point in the history
  • Loading branch information
spaced4ndy authored Dec 5, 2024
1 parent 966b999 commit 9893935
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 11 deletions.
17 changes: 11 additions & 6 deletions src/Simplex/Messaging/Agent.hs
Original file line number Diff line number Diff line change
Expand Up @@ -687,9 +687,15 @@ allowConnectionAsync' c corrId connId confId ownConnInfo =
enqueueCommand c corrId connId (Just server) $ AClientCommand $ LET confId ownConnInfo
_ -> throwE $ CMD PROHIBITED "allowConnectionAsync"

-- TODO
-- Unlike `acceptContact` (synchronous version), `acceptContactAsync` uses `unacceptInvitation` in case of error,
-- because we're not taking lock here. In practice it is less likely to fail because it doesn't involve network IO,
-- and also it can't be triggered by user concurrently several times in a row. It could be improved similarly to
-- `acceptContact` by creating a new map for invitation locks and taking lock here, and removing `unacceptInvitation`
-- while marking invitation as accepted inside "lock level transaction" after successful `joinConnAsync`.
acceptContactAsync' :: AgentClient -> ACorrId -> Bool -> InvitationId -> ConnInfo -> PQSupport -> SubscriptionMode -> AM ConnId
acceptContactAsync' c corrId enableNtfs invId ownConnInfo pqSupport subMode = do
Invitation {contactConnId, connReq} <- withStore c (`getInvitation` invId)
Invitation {contactConnId, connReq} <- withStore c $ \db -> getInvitation db "acceptContactAsync'" invId
withStore c (`getConn` contactConnId) >>= \case
SomeConn _ (ContactConnection ConnData {userId} _) -> do
withStore' c $ \db -> acceptInvitation db invId ownConnInfo
Expand Down Expand Up @@ -809,7 +815,7 @@ newConnToJoin c userId connId enableNtfs cReq pqSup = case cReq of

newConnToAccept :: AgentClient -> ConnId -> Bool -> ConfirmationId -> PQSupport -> AM ConnId
newConnToAccept c connId enableNtfs invId pqSup = do
Invitation {connReq, contactConnId} <- withStore c (`getInvitation` invId)
Invitation {connReq, contactConnId} <- withStore c $ \db -> getInvitation db "newConnToAccept" invId
withStore c (`getConn` contactConnId) >>= \case
SomeConn _ (ContactConnection ConnData {userId} _) ->
newConnToJoin c userId connId enableNtfs connReq pqSup
Expand Down Expand Up @@ -941,13 +947,12 @@ allowConnection' c connId confId ownConnInfo = withConnLock c connId "allowConne
-- | Accept contact (ACPT command) in Reader monad
acceptContact' :: AgentClient -> ConnId -> Bool -> InvitationId -> ConnInfo -> PQSupport -> SubscriptionMode -> AM SndQueueSecured
acceptContact' c connId enableNtfs invId ownConnInfo pqSupport subMode = withConnLock c connId "acceptContact" $ do
Invitation {contactConnId, connReq} <- withStore c (`getInvitation` invId)
Invitation {contactConnId, connReq} <- withStore c $ \db -> getInvitation db "acceptContact'" invId
withStore c (`getConn` contactConnId) >>= \case
SomeConn _ (ContactConnection ConnData {userId} _) -> do
sqSecured <- joinConn c userId connId enableNtfs connReq ownConnInfo pqSupport subMode
withStore' c $ \db -> acceptInvitation db invId ownConnInfo
joinConn c userId connId enableNtfs connReq ownConnInfo pqSupport subMode `catchAgentError` \err -> do
withStore' c (`unacceptInvitation` invId)
throwE err
pure sqSecured
_ -> throwE $ CMD PROHIBITED "acceptContact"

-- | Reject contact (RJCT command) in Reader monad
Expand Down
2 changes: 1 addition & 1 deletion src/Simplex/Messaging/Agent/Client.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2020,7 +2020,7 @@ storeError = \case
SEConnDuplicate -> CONN DUPLICATE
SEBadConnType CRcv -> CONN SIMPLEX
SEBadConnType CSnd -> CONN SIMPLEX
SEInvitationNotFound -> CMD PROHIBITED "SEInvitationNotFound"
SEInvitationNotFound cxt invId -> CMD PROHIBITED $ "SEInvitationNotFound " <> cxt <> ", invitationId = " <> show invId
-- this error is never reported as store error,
-- it is used to wrap agent operations when "transaction-like" store access is needed
-- NOTE: network IO should NOT be used inside AgentStoreMonad
Expand Down
2 changes: 1 addition & 1 deletion src/Simplex/Messaging/Agent/Store.hs
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ data StoreError
| -- | Confirmation not found.
SEConfirmationNotFound
| -- | Invitation not found
SEInvitationNotFound
SEInvitationNotFound String InvitationId
| -- | Message not found
SEMsgNotFound
| -- | Command not found
Expand Down
6 changes: 3 additions & 3 deletions src/Simplex/Messaging/Agent/Store/SQLite.hs
Original file line number Diff line number Diff line change
Expand Up @@ -926,9 +926,9 @@ createInvitation db gVar NewInvitation {contactConnId, connReq, recipientConnInf
|]
(invitationId, contactConnId, connReq, recipientConnInfo)

getInvitation :: DB.Connection -> InvitationId -> IO (Either StoreError Invitation)
getInvitation db invitationId =
firstRow invitation SEInvitationNotFound $
getInvitation :: DB.Connection -> String -> InvitationId -> IO (Either StoreError Invitation)
getInvitation db cxt invitationId =
firstRow invitation (SEInvitationNotFound cxt invitationId) $
DB.query
db
[sql|
Expand Down

0 comments on commit 9893935

Please sign in to comment.