mirror of https://github.com/zulip/zulip.git
003fa7adda
Fundamentally, we should take a write lock on the message, check its validity for a change, and then make and commit that change. Previously, `check_update_message` did not operate in a transaction, but `do_update_message` did -- which led to the ordering: - `check_update_message` reads Message, not in a transaction - `check_update_message` verifies properties of the Message - `do_update_message` starts a transaction - `do_update_message` takes a read lock on UserMessage - `do_update_message` writes on UserMessage - `do_update_message` writes Message - `do_update_message` commits This leads to race conditions, where the `check_update_message` may have verified based on stale data, and `do_update_message` may improperly overwrite it; as well as deadlocks, where other (properly-written) codepaths take a write lock on Message _before_ updating UserMessage, and thus deadlock with `do_update_message`. Change `check_update_message` to open a transaction, and take the write lock when first accessing the Message row. We update the comment above `do_update_message` to clarify this expectation. The new ordering is thus: - `check_update_message` starts a transaction - `check_update_message` takes a write lock on Message - `check_update_message` verifies properties of the Message - `do_update_message` writes on UserMessage - `do_update_message` writes Message - `check_update_message` commits |
||
|---|---|---|
| .. | ||
| actions | ||
| data_import | ||
| integration_fixtures/nagios | ||
| lib | ||
| management | ||
| migrations | ||
| openapi | ||
| tests | ||
| tornado | ||
| transaction_tests | ||
| views | ||
| webhooks | ||
| worker | ||
| __init__.py | ||
| apps.py | ||
| context_processors.py | ||
| decorator.py | ||
| filters.py | ||
| forms.py | ||
| logging_handlers.py | ||
| middleware.py | ||
| models.py | ||
| signals.py | ||