From 53e330dac9da8684b329fc5f7053cccbcbe1431e Mon Sep 17 00:00:00 2001 From: JRoberts <8711996+jr-simplex@users.noreply.github.com> Date: Thu, 12 May 2022 17:37:09 +0400 Subject: [PATCH] core: add missing status transitions for group file transfer; fix group file delivery race condition (#640) --- docs/rfcs/files.mmd | 129 ++++++++++++++++++++++++++++++++++++++ src/Simplex/Chat.hs | 18 +++--- src/Simplex/Chat/Store.hs | 10 +-- 3 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 docs/rfcs/files.mmd diff --git a/docs/rfcs/files.mmd b/docs/rfcs/files.mmd new file mode 100644 index 0000000000..4431726b77 --- /dev/null +++ b/docs/rfcs/files.mmd @@ -0,0 +1,129 @@ +sequenceDiagram + participant AU as Alice's
chat UI + participant AC as Alice's
chat core + participant AA as Alice's
agent + participant BA as Bob's
agent + participant BC as Bob's
chat + participant BU as Bob's
chat UI + + note over AU, AA: Alice's app + note over BA, BU: Bob's app + + note over AU, BU: Direct file transfer handshake + + note over AU: Send message with file + AU ->> AC: APISendMessage with filePath + AC ->> AA: createConnection + AA ->> AC: connId, fileConnReq + note left of AC: createSndFileTransfer
files.ci_file_status = CIFSSndStored
snd_files.file_status = FSNew
connections.conn_type = ConnSndFile
connections.conn_status = ConnNew + AC ->> BC: XMsgNew with FileInvitation, fileConnReq = fileConnReq + note right of BC: createRcvFileTransfer
files.ci_file_status = CIFSRcvInvitation
rcv_files.file_status = FSNew
rcv_files.file_queue_info = fileConnReq + BC ->> BU: CRNewChatItem + note over BU: Accept file + BU ->> BC: ReceiveFile + BC ->> BA: joinConnection with ConnInfo = XFileAcpt + BA ->> BC: connId + note right of BC: acceptRcvFileTransfer
files.ci_file_status = CIFSRcvAccepted
rcv_files.file_status = FSAccepted
connections.conn_type = ConnRcvFile
connections.conn_status = ConnJoined + BA ->> AC: CONF with XFileAcpt + note left of AC: snd_files.file_status = FSAccepted + AC ->> AA: allowConnection + note left of AC: connections.conn_status = ConnAccepted + note over AA, BA: ...
Connection handshake
... + par Alice connected + AA ->> AC: CON + note left of AC: connections.conn_status = ConnReady
snd_files.file_status = FSConnected
files.ci_file_status = CIFSSndTransfer + AC ->> AU: CRSndFileStart + note over AC: sendFileChunk + and Bob Connected + BA ->> BC: CON + note right of BC: connections.conn_status = ConnReady
rcv_files.file_status = FSConnected
files.ci_file_status = CIFSRcvTransfer + BC ->> BU: CRRcvFileStart + end + + note over AU, BU: Group file transfer handshake + + note over AU: Send message with file + AU ->> AC: APISendMessage with filePath + note left of AC: createSndGroupFileTransfer
files.ci_file_status = CIFSSndStored + AC ->> BC: XMsgNew with FileInvitation, fileConnReq = Nothing + note right of BC: createRcvGroupFileTransfer
files.ci_file_status = CIFSRcvInvitation
rcv_files.file_status = FSNew
rcv_files.file_queue_info = NULL + BC ->> BU: CRNewChatItem + note over BU: Accept file + BU ->> BC: ReceiveFile + BC ->> BA: createConnection + BA ->> BC: connId, fileInvConnReq + note right of BC: acceptRcvFileTransfer
files.ci_file_status = CIFSRcvAccepted
rcv_files.file_status = FSAccepted
connections.conn_type = ConnRcvFile
connections.conn_status = ConnNew + BC ->> AC: XFileAcptInv sharedMsgId fileInvConnReq + AC ->> AA: joinConnection with ConnInfo = XOk + AA ->> AC: connId + note left of AC: createSndGroupFileTransferConnection
connections.conn_type = ConnSndFile
connections.conn_status = ConnNew
snd_files.file_status = FSAccepted + AA ->> BC: CONF with XOk + BC ->> BA: allowConnection + note right of BC: connections.conn_status = ConnAccepted + note over AA, BA: ...
Connection handshake
... + par Alice connected + AA ->> AC: CON + note left of AC: connections.conn_status = ConnReady
snd_files.file_status = FSConnected
files.ci_file_status = CIFSSndTransfer + AC ->> AU: CRSndFileStart + note over AC: sendFileChunk + and Bob Connected + BA ->> BC: CON + note right of BC: connections.conn_status = ConnReady
rcv_files.file_status = FSConnected
files.ci_file_status = CIFSRcvTransfer + BC ->> BU: CRRcvFileStart + end + + note over AU, BU: File transfer + + loop while createSndFileChunk returns chunkNo + AC ->> BC: FileChunk + AA ->> AC: SENT + note over BC: appendFileChunk + end + opt receiver cancelled file transfer + note over BU: Cancel file + BU ->> BC: CancelFile + note right of BC: files.cancelled = true
files.ci_file_status = CIFSRcvCancelled
rcv_files.file_status = FSCancelled
deleteRcvFileChunks + BC ->> BA: deleteConnection + BC ->> BU: CRRcvFileSndCancelled + note over AA, BA: connection is deleted + AC ->> BA: FileChunk + BA ->> AA: MERR AUTH + AA ->> AC: MERR + note over AC: cancelSndFileTransfer + alt AUTH + AC ->> AU: CRSndFileRcvCancelled + else other error, possibly not due to cancel + AC ->> AU: CEFileSend + end + end + AC ->> BC: Final FileChunk + note left of AC: snd_files.file_status = FSComplete
deleteSndFileChunks
files.ci_file_status = CIFSSndComplete + AC ->> AU: CRSndFileComplete + AC ->> AA: deleteConnection + note over BC: appendFileChunk + note right of BC: rcv_files.file_status = FSComplete
files.ci_file_status = CIFSRcvComplete
deleteRcvFileChunks + BC ->> BU: CRRcvFileComplete + BC ->> BA: deleteConnection + + note over AU, BU: Sender cancels file transfer + + note over AU: Cancel file + AU ->> AC: CancelFile + note left of AC: files.cancelled = true
files.ci_file_status = CIFSSndCancelled
snd_files.file_status = FSCancelled
deleteSndFileChunks + AC ->> BA: FileChunkCancel (over file connection) + note left of AC: deleteConnection + AC ->> BA: XFileCancel (over direct/group connection) + AC ->> AU: CRSndGroupFileCancelled + par FileChunkCancel + BA ->> BC: FileChunkCancel + note over BC: Cancel file (if it wasn't already cancelled) + note right of BC: files.cancelled = true
files.ci_file_status = CIFSRcvCancelled
rcv_files.file_status = FSCancelled
deleteRcvFileChunks + BC ->> BA: deleteConnection + BC ->> BU: CRRcvFileSndCancelled + and XFileCancel + BA ->> BC: XFileCancel + note over BC: Cancel file (if it wasn't already cancelled) + note right of BC: files.cancelled = true
files.ci_file_status = CIFSRcvCancelled
rcv_files.file_status = FSCancelled
deleteRcvFileChunks + BC ->> BA: deleteConnection + BC ->> BU: CRRcvFileSndCancelled + end diff --git a/src/Simplex/Chat.hs b/src/Simplex/Chat.hs index 2bd6bdfd00..bbc8000c28 100644 --- a/src/Simplex/Chat.hs +++ b/src/Simplex/Chat.hs @@ -842,7 +842,7 @@ acceptFileReceive user@User {userId} RcvFileTransfer {fileId, fileInvitation = F tryError (withAgent $ \a -> joinConnection a connReq . directMessage $ XFileAcpt fName) >>= \case Right agentConnId -> do filePath <- getRcvFilePath filePath_ fName - withStore $ \st -> acceptRcvFileTransfer st user fileId agentConnId filePath + withStore $ \st -> acceptRcvFileTransfer st user fileId agentConnId ConnJoined filePath Left e -> throwError e -- group file protocol Nothing -> @@ -855,7 +855,7 @@ acceptFileReceive user@User {userId} RcvFileTransfer {fileId, fileInvitation = F sharedMsgId <- withStore $ \st -> getSharedMsgIdByFileId st userId fileId (agentConnId, fileInvConnReq) <- withAgent (`createConnection` SCMInvitation) filePath <- getRcvFilePath filePath_ fName - ci <- withStore $ \st -> acceptRcvFileTransfer st user fileId agentConnId filePath + ci <- withStore $ \st -> acceptRcvFileTransfer st user fileId agentConnId ConnNew filePath void $ sendDirectMessage conn (XFileAcptInv sharedMsgId fileInvConnReq fName) (GroupId groupId) pure ci _ -> throwChatError $ CEFileInternal "member connection not active" @@ -1836,16 +1836,18 @@ parseFileChunk msg = appendFileChunk :: ChatMonad m => RcvFileTransfer -> Integer -> ByteString -> m () appendFileChunk ft@RcvFileTransfer {fileId, fileStatus} chunkNo chunk = case fileStatus of - RFSConnected RcvFileInfo {filePath} -> do - fsFilePath <- toFSFilePath filePath - append_ filePath fsFilePath + RFSConnected RcvFileInfo {filePath} -> append_ filePath + -- sometimes update of file transfer status to FSConnected + -- doesn't complete in time before MSG with first file chunk + RFSAccepted RcvFileInfo {filePath} -> append_ filePath RFSCancelled _ -> pure () _ -> throwChatError $ CEFileInternal "receiving file transfer not in progress" where - append_ fPath fPathUsed = do - h <- getFileHandle fileId fPathUsed rcvFiles AppendMode + append_ filePath = do + fsFilePath <- toFSFilePath filePath + h <- getFileHandle fileId fsFilePath rcvFiles AppendMode E.try (liftIO $ B.hPut h chunk >> hFlush h) >>= \case - Left (e :: E.SomeException) -> throwChatError . CEFileWrite fPath $ show e + Left (e :: E.SomeException) -> throwChatError . CEFileWrite fsFilePath $ show e Right () -> withStore $ \st -> updatedRcvFileChunkStored st ft chunkNo getFileHandle :: ChatMonad m => Int64 -> FilePath -> (ChatController -> TVar (Map Int64 Handle)) -> IOMode -> m Handle diff --git a/src/Simplex/Chat/Store.hs b/src/Simplex/Chat/Store.hs index 30e8f8a4d2..c08406646a 100644 --- a/src/Simplex/Chat/Store.hs +++ b/src/Simplex/Chat/Store.hs @@ -2025,8 +2025,8 @@ createRcvGroupFileTransfer st userId GroupMember {groupId, groupMemberId, localD currentTs <- getCurrentTime DB.execute db - "INSERT INTO files (user_id, group_id, file_name, file_size, chunk_size, created_at, updated_at) VALUES (?,?,?,?,?,?,?)" - (userId, groupId, fileName, fileSize, chunkSize, currentTs, currentTs) + "INSERT INTO files (user_id, group_id, file_name, file_size, chunk_size, ci_file_status, created_at, updated_at) VALUES (?,?,?,?,?,?,?,?)" + (userId, groupId, fileName, fileSize, chunkSize, CIFSRcvInvitation, currentTs, currentTs) fileId <- insertedRowId db DB.execute db @@ -2082,8 +2082,8 @@ getRcvFileTransfer_ db userId fileId = cancelled = fromMaybe False cancelled_ rcvFileTransfer _ = Left $ SERcvFileNotFound fileId -acceptRcvFileTransfer :: StoreMonad m => SQLiteStore -> User -> Int64 -> ConnId -> FilePath -> m AChatItem -acceptRcvFileTransfer st user@User {userId} fileId agentConnId filePath = +acceptRcvFileTransfer :: StoreMonad m => SQLiteStore -> User -> Int64 -> ConnId -> ConnStatus -> FilePath -> m AChatItem +acceptRcvFileTransfer st user@User {userId} fileId agentConnId connStatus filePath = liftIOEither . withTransaction st $ \db -> do currentTs <- getCurrentTime DB.execute @@ -2097,7 +2097,7 @@ acceptRcvFileTransfer st user@User {userId} fileId agentConnId filePath = DB.execute db "INSERT INTO connections (agent_conn_id, conn_status, conn_type, rcv_file_id, user_id, created_at, updated_at) VALUES (?,?,?,?,?,?,?)" - (agentConnId, ConnJoined, ConnRcvFile, fileId, userId, currentTs, currentTs) + (agentConnId, connStatus, ConnRcvFile, fileId, userId, currentTs, currentTs) getChatItemByFileId_ db user fileId updateRcvFileStatus :: MonadUnliftIO m => SQLiteStore -> RcvFileTransfer -> FileStatus -> m ()