Skip to content

[stable26] Avoid cleanup on close to not run into race conditions with other sessions on the same document #3922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
juliusknorr merged 10 commits into from
Mar 9, 2023
Merged

[stable26] Avoid cleanup on close to not run into race conditions with other sessions on the same document #3922

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fcb3ce8
feat(logging): Add debug logging for empty file saving
juliusknorr Mar 7, 2023
2dd921d
fix: Rework save locking
juliusknorr Mar 7, 2023
a5b9229
feat(logging): Add some logging to document cleanup
juliusknorr Mar 7, 2023
9754f1f
fix: Do not cleanup on close
juliusknorr Mar 7, 2023
4eac11d
fix: Cleanup attachments on itinial document cleanup
juliusknorr Mar 7, 2023
949b404
fix: Still unlock documents on close
juliusknorr Mar 8, 2023
dba594d
chore: Make resetDocument more readable
juliusknorr Mar 8, 2023
45662b5
fix: Do not reset on create
juliusknorr Mar 8, 2023
a3c6fb8
fix: Cleanup actual documents in cron
juliusknorr Mar 8, 2023
b38032e
ci: Adapt cypress tests to new cleanup behaviour
juliusknorr Mar 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: Do not cleanup on close 
Signed-off-by: Julius Härtl <[email protected]>
  • Loading branch information
@juliusknorr @backportbot-nextcloud
juliusknorr authored and backportbot-nextcloud[bot] committed Mar 9, 2023
commit 9754f1f8e0aad8407ddf66a7fe806adec00d6b45
2 changes: 1 addition & 1 deletion lib/Cron/Cleanup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ protected function run($argument) {
$this->logger->error('Document ' . $session->getDocumentId() . ' has not been reset, as an error occured', ['exception' => $e]);
}
}
$removedSessions = $this->sessionService->removeInactiveSessions(null);
$removedSessions = $this->sessionService->removeInactiveSessionsWithoutSteps(null);
$this->logger->debug('Removed ' . $removedSessions . ' inactive sessions');
}
}
2 changes: 1 addition & 1 deletion lib/Db/SessionMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public function findAllInactive() {
return $this->findEntities($qb);
}

public function deleteInactive($documentId = -1) {
public function deleteInactiveWithoutSteps($documentId = -1) {
$qb = $this->db->getQueryBuilder();
$qb->select('session_id')
->from('text_steps');
Expand Down
14 changes: 2 additions & 12 deletions lib/Service/ApiService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa

$readOnly = $this->documentService->isReadOnly($file, $token);

$this->sessionService->removeInactiveSessions($file->getId());
$this->sessionService->removeInactiveSessionsWithoutSteps($file->getId());
$remainingSessions = $this->sessionService->getAllSessions($file->getId());
$freshSession = false;
if ($forceRecreate || count($remainingSessions) === 0) {
Expand Down Expand Up @@ -183,17 +183,7 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa

public function close($documentId, $sessionId, $sessionToken): DataResponse {
$this->sessionService->closeSession($documentId, $sessionId, $sessionToken);
$this->sessionService->removeInactiveSessions($documentId);
$activeSessions = $this->sessionService->getActiveSessions($documentId);
if (count($activeSessions) === 0) {
try {
$this->documentService->resetDocument($documentId);
$this->attachmentService->cleanupAttachments($documentId);
$this->logger->info('Reset unsaved changes of ' . $documentId);
} catch (DocumentHasUnsavedChangesException $e) {
$this->logger->error('Did not reset unsaved changes during close of ' . $documentId, ['exception' => $e]);
}
}
$this->sessionService->removeInactiveSessionsWithoutSteps($documentId);
return new DataResponse([]);
}

Expand Down
4 changes: 2 additions & 2 deletions lib/Service/SessionService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ public function findAllInactive() {
return $this->sessionMapper->findAllInactive();
}

public function removeInactiveSessions($documentId = -1) {
public function removeInactiveSessionsWithoutSteps($documentId = -1) {
// No need to clear the cache here as we already set a TTL
return $this->sessionMapper->deleteInactive($documentId);
return $this->sessionMapper->deleteInactiveWithoutSteps($documentId);
}

/**
Expand Down