mirror of
https://github.com/BookStackApp/BookStack.git
synced 2026-02-07 11:19:38 +03:00
Race conditions lead to errors for content permission generation #4455
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @matthias4217 on GitHub (Feb 12, 2024).
Describe the Bug
To edit pages on Bookstack, some people in our team use the web interface, while others store their content on git and use a Python tool to upload it to Bookstack. In the latter case, this has caused some permission issues, with permissions on the book disappearing (but not on the pages or chapters). The
joint_permissionstable is then empty for the targeted books. Manually creating permissions in this table, or usingphp artisan bookstack:regenerate-permissionsfixes the issue. This has happened three times yet, the first time on January 23rd. It had never happened before we’ve used 23.12. However we are increasingly using Bookstack and the issue could have been present before.It seems similar to https://github.com/BookStackApp/BookStack/issues/4791, but on the book, and not on the page.
Steps to Reproduce
I am not yet sure how the bug is caused exactly. What I can say is that it has always happened while using the API.
Expected Behaviour
The book should remain accessible.
Screenshots or Additional Context
No response
Browser Details
No response
Exact BookStack Version
v23.12
@ssddanbrown commented on GitHub (Feb 12, 2024):
Hi @matthias4217, Could you confirm specifically what API endpoints are being used, and the order they're used in?
Or (if public) link to the tool and advise which methods/actions of the tool are being used?
Just want to gain an idea of exactly what endpoints are used.
@matthias4217 commented on GitHub (Feb 16, 2024):
We have developed two tools around Bookstack. Both are Python apps, in private repositories.
get,putandpostfrom requests.Sessionobject from requests.The error has been detected using Gitplo on the first call to the Bookstack API, a
GETon the books endpoint. I suppose the issue began before that. The three impacted books are edited using Gitplo, and only two of them are managed by Bookstack Carpenter, so I assume it’s a call from Gitplo that is responsible for the issue. These books are notably not modified through the web interface.Gitplo uses these API endpoints :
GET,LIST,DELETE,CREATEGET,LISTGET,LIST,DELETE,CREATE,UPDATEGET,LIST,DELETE,CREATEGET,LIST,DELETE,CREATE,UPDATENotably, it doesn’t modify books nor does it use the Content-permissions endpoint, and apart for three times where this error has happened (on two Bookstack instances in total), our worflow works fine.
@ssddanbrown commented on GitHub (Feb 22, 2024):
Thanks for the extra info @matthias4217.
Can you detail how you're running BookStack? (hosting method, OS, webserver).
@matthias4217 commented on GitHub (Feb 26, 2024):
We run Bookstack on Openshift, and we build our Docker images ourselves (with
php:8.1-fpm-alpineandnginx:1.25-alpine). We have slightly modified the code to show resource ids on pages (using this merge request : https://github.com/BookStackApp/BookStack/pull/4314/files) and tot display the book parent shelves (from this issue : https://github.com/BookStackApp/BookStack/issues/4547#issuecomment-1726420230).@ssddanbrown commented on GitHub (Mar 16, 2024):
Report from a user on discord of similar, running on Ubuntu 18.04 server, with MariaDB 10.1.48 database, Apache2, & PHP8.1-CGI. Reference
@zivillian commented on GitHub (Nov 8, 2024):
We have a similar problem with missing page permission. We are still trying to reproduce, but our assumption is that calling
PUT /api/pages/{id}for multiple pages with changedpriorityin parallel was the root cause (we tried to sort the book).After
php artisan bookstack:regenerate-permissionsthe pages reappeared.System info: BookStack 24.10 on Ubuntu 24.04.1 with MariaDB 10.11.8 and libapache2-mod-php8.3
@zivillian commented on GitHub (Nov 8, 2024):
I was able to reproduce the issue and I think I found the issue. When multiple API requests are running simultaneously there is a race condition. The permissions are first deleted and then recreated:
ac27e18933/app/Permissions/JointPermissionBuilder.php (L146-L151)Delete and Create are each wrapped in transaction:
ac27e18933/app/Permissions/JointPermissionBuilder.php (L163)ac27e18933/app/Permissions/JointPermissionBuilder.php (L228)But two parallel update requests for different pages within the same book may interleave and fail with missing permissions for the second page. Both requests try to update the permissions for the page and the containing book:
ac27e18933/app/Permissions/JointPermissionBuilder.php (L54-L80)Example flow:
Stacktrace:
@ssddanbrown commented on GitHub (Jul 1, 2025):
Another report via #5686.
Thanks for the info @zivillian.
I've assigned this to the next feature release to be addressed.
Needs some careful consideration in regards to the logical order, with consideration to the exact locking/access methods of MySQL, to ensure it's fail-safe and that we won't be rolling back (via transactions) permission changes out of sync with the UI. Right now the deletions are specifically done first in their own transactions to somewhat achieve this. We could regen then delete+insert in one transaction but I need to double check the potential parallel logic.
Could also improve permission rebuild targeting to be more specific (right now it'll re-assess the whole parent chain since that info is needed for assessment during generation) but that's maybe something for a future improvement.
@ssddanbrown commented on GitHub (Jul 7, 2025):
Some changes now prepared in #5689.
These changes wrap the permission-affecting actions as a whole, at a higher level, i transactions.
This naturally provides blocking of edits to the permission store during generation to prevent attempted parallel changes, while also providing roll-back of the entire general action (page creation, permission change etc...) in the event something goes wrong.
I found that the default MySQL "REPEATABLE READ" transaction logic could probably be problematic for BookStack kind of scenarios (since, for example, a later parallel process may end up generating permissions based on old information) so we akwardly go out of the way to set instead use "READ COMMITTED" transactions.
@ssddanbrown commented on GitHub (Jul 9, 2025):
#5689 Has now been merged, to be part of the next feature release, so I will therefore close this off.
Thanks again @zivillian @matthias4217 for your reports and provided detail.