From 63970c74d292006bd7dd519ba68b0de3c57f8a00 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:18:00 +0000 Subject: [PATCH 1/6] Bump django from 4.2.30 to 5.2.16 Bumps [django](https://github.com/django/django) from 4.2.30 to 5.2.16. - [Commits](https://github.com/django/django/compare/4.2.30...5.2.16) --- updated-dependencies: - dependency-name: django dependency-version: 5.2.16 dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d38836b..c947253 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ packages = [] name = "simone" version = "0.0.1" dependencies = [ - "Django>=4.0.8,<5.0", + "Django>=4.0.8,<6.0", "cron-validator>=1.0.3", "cryptography>=43.0.0", "gunicorn>=20.1.0", diff --git a/requirements.txt b/requirements.txt index 3942e21..40602c3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,7 +10,7 @@ charset-normalizer==3.4.7 click==8.4.2 cron-validator==1.0.8 cryptography==49.0.0 -django==4.2.30 +django==5.2.16 django-debug-toolbar==6.3.0 exceptiongroup==1.3.1; python_version=='3.10' gunicorn==26.0.0 From 6c2537e63b58a8d47210a7bb4fcdf32ad88ca7d7 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Aug 2026 10:37:21 -0700 Subject: [PATCH 2/6] Fix handler_about.Fact for Django 5.2's removal of index_together Django 5.1 dropped support for the Meta.index_together option, which broke app startup on this branch's Django 5.2 bump (TypeError: 'class Meta' got invalid attribute(s): index_together). Replace it with the modern Meta.indexes API. The migration can't just rename the old index in place: the composite index_together index was already silently lost on SQLite because 0003_make_workspace_nonnull's AlterField rebuilds the whole table, and SQLite's table-rebuild path only restores indexes tracked via Meta.indexes, not the legacy index_together. On MySQL (production) the AlterField is an in-place ALTER TABLE, so the old index is untouched and still physically present under its old auto-generated name. The new migration accounts for both cases: it drops the old index by its (deterministic, backend- independent) name if found, then adds the new named index. --- .../migrations/0004_replace_index_together.py | 61 +++++++++++++++++++ handler_about/models.py | 2 +- 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 handler_about/migrations/0004_replace_index_together.py diff --git a/handler_about/migrations/0004_replace_index_together.py b/handler_about/migrations/0004_replace_index_together.py new file mode 100644 index 0000000..345428f --- /dev/null +++ b/handler_about/migrations/0004_replace_index_together.py @@ -0,0 +1,61 @@ +from django.db import migrations, models + +# Django 5.1 removed the `index_together` Meta option, so the model now +# declares this same composite index via `Meta.indexes` instead. The name +# below is the one Django's schema editor deterministically assigned to the +# old index_together-based index (a hash of the table + column names, so +# it's the same on every backend) when it was created back in +# 0002_add_workspace_fk. On SQLite it no longer actually exists — the +# 0003_make_workspace_nonnull AlterField rebuilt the table, and SQLite's +# table-rebuild only restores indexes tracked via Meta.indexes, silently +# dropping the index_together one — but on MySQL it's still present, so we +# look for it by name and drop it if we find it. +LEGACY_INDEX_NAME = ( + 'handler_about_fact_workspace_id_user_id_created_at_cff0afa0_idx' +) +NEW_INDEX_NAME = 'handler_abo_workspa_4d097e_idx' + + +def drop_legacy_index(apps, schema_editor): + Fact = apps.get_model('handler_about', 'Fact') + table = Fact._meta.db_table + with schema_editor.connection.cursor() as cursor: + constraints = schema_editor.connection.introspection.get_constraints( + cursor, table + ) + if LEGACY_INDEX_NAME in constraints: + schema_editor.remove_index(Fact, models.Index(name=LEGACY_INDEX_NAME)) + + +class Migration(migrations.Migration): + + dependencies = [('handler_about', '0003_make_workspace_nonnull')] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.AlterIndexTogether( + name='fact', index_together=set() + ), + migrations.AddIndex( + model_name='fact', + index=models.Index( + fields=['workspace', 'user_id', 'created_at'], + name=NEW_INDEX_NAME, + ), + ), + ], + database_operations=[ + migrations.RunPython( + drop_legacy_index, migrations.RunPython.noop + ), + migrations.AddIndex( + model_name='fact', + index=models.Index( + fields=['workspace', 'user_id', 'created_at'], + name=NEW_INDEX_NAME, + ), + ), + ], + ) + ] diff --git a/handler_about/models.py b/handler_about/models.py index ba94c42..e440940 100644 --- a/handler_about/models.py +++ b/handler_about/models.py @@ -13,6 +13,6 @@ def __str__(self): return f'{self.id} - {self.value}' class Meta: - index_together = (('workspace', 'user_id', 'created_at'),) + indexes = [models.Index(fields=['workspace', 'user_id', 'created_at'])] unique_together = (('workspace', 'user_id', 'value'),) ordering = ('user_id', 'created_at') From 15daa3cf3af14df57c29208cc216cb07509e0aa8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Aug 2026 10:39:48 -0700 Subject: [PATCH 3/6] Fix Index() construction in the index_together migration models.Index() validates that fields/expressions are non-empty in its constructor, even though remove_index()/remove_sql() only actually need the .name to build the DROP INDEX statement. My local SQLite testing didn't exercise this: the legacy index doesn't physically exist there (see the previous commit's message), so drop_legacy_index() short-circuited before constructing the Index. Production's MySQL DB does still have it, so it hit the construction and raised ValueError on startup. Verified by manually recreating the legacy index on a SQLite DB at migration 0003 and confirming 0004 now drops it and adds the new one. --- handler_about/migrations/0004_replace_index_together.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/handler_about/migrations/0004_replace_index_together.py b/handler_about/migrations/0004_replace_index_together.py index 345428f..29b201e 100644 --- a/handler_about/migrations/0004_replace_index_together.py +++ b/handler_about/migrations/0004_replace_index_together.py @@ -24,7 +24,13 @@ def drop_legacy_index(apps, schema_editor): cursor, table ) if LEGACY_INDEX_NAME in constraints: - schema_editor.remove_index(Fact, models.Index(name=LEGACY_INDEX_NAME)) + schema_editor.remove_index( + Fact, + models.Index( + fields=['workspace', 'user_id', 'created_at'], + name=LEGACY_INDEX_NAME, + ), + ) class Migration(migrations.Migration): From 34a3d68df69e3c437424128b9c0e259c77f989bf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Aug 2026 10:51:08 -0700 Subject: [PATCH 4/6] Fix hung requests (nginx 499s) from gunicorn --preload sharing a DB socket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit script/run launches gunicorn with --preload, which imports simone.urls (and therefore starts the Cron background thread, which opens a DB connection on its first tick) in the master process before workers are forked. fork() duplicates the master's open MySQL socket fd into the worker, so the master's Cron thread and the worker end up sharing one underlying connection. Once both sides use it concurrently they corrupt each other's protocol state on that socket and hang forever waiting on a response that will never arrive — no exception, no log line, just a request that never completes until Slack's ~3s client timeout gives up (nginx logs this as a 499, since upstream never responded). Add a post_fork gunicorn hook that drops any DB connections inherited from the master, forcing each worker to lazily open its own independent connection on first use. This is the standard fix for gunicorn --preload combined with persistent database connections (CONN_MAX_AGE=300 here). Verified locally: gunicorn boots with the new config and post_fork hook runs without error, and a DB-touching request (/adm/login/) completes in ~20-30ms with no hang. Couldn't reproduce the exact master/worker socket race end-to-end without MySQL access in this environment, but the mechanism matches the observed symptoms precisely and this is the documented remedy for it. --- gunicorn.conf.py | 14 ++++++++++++++ script/run | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 gunicorn.conf.py diff --git a/gunicorn.conf.py b/gunicorn.conf.py new file mode 100644 index 0000000..5099d46 --- /dev/null +++ b/gunicorn.conf.py @@ -0,0 +1,14 @@ +def post_fork(server, worker): + ''' + We run with --preload, so `simone.urls` (and the Cron thread it starts) + is imported in the master process before workers are forked. Any + database connection the master has opened by that point (e.g. Cron's + first tick) gets duplicated into each worker via fork(), leaving the + master and worker sharing one underlying socket. Once both sides use it + concurrently they corrupt each other's protocol state and hang forever + with no error logged. Dropping inherited connections here forces each + worker to lazily open its own on first use instead. + ''' + from django.db import connections + + connections.close_all() diff --git a/script/run b/script/run index b79fad6..709b681 100755 --- a/script/run +++ b/script/run @@ -4,4 +4,4 @@ set -e ./manage.py collectstatic --no-input ./manage.py migrate --no-input -gunicorn simone.wsgi --bind 0.0.0.0:6444 --graceful-timeout 5 --preload --threads 4 +gunicorn simone.wsgi --config gunicorn.conf.py --bind 0.0.0.0:6444 --graceful-timeout 5 --preload --threads 4 From 826dc7498cec915af3ac197527045620670f4d02 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Aug 2026 10:53:23 -0700 Subject: [PATCH 5/6] Include gunicorn.conf.py in the Docker build context .dockerignore excludes everything by default and only allowlists specific paths back in. gunicorn.conf.py (added in the previous commit for the post_fork DB-connection fix) was never added to that allowlist, so it never made it into the image and script/run's --config gunicorn.conf.py failed with "Error: 'gunicorn.conf.py' doesn't exist". --- .dockerignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.dockerignore b/.dockerignore index d28b308..09bb957 100644 --- a/.dockerignore +++ b/.dockerignore @@ -2,6 +2,7 @@ * # Explicitly include the bits we want # TODO: find a good way to keep this in sync +!gunicorn.conf.py !handler/*.py !handler/chat/*.py !handler/management/*.py From 9bdbdbe9ca5d4746caf5a75666c5c8c047127ec3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Aug 2026 11:01:30 -0700 Subject: [PATCH 6/6] Drop gunicorn --preload; it's the actual source of the request hangs The post_fork connections.close_all() from the last commit turned out to be a no-op for this: Django's ConnectionHandler keys connections by thread-local storage, and post_fork runs on the thread that called fork() (the arbiter's), not the Cron thread that actually opened the connection in the master. So there was nothing in that thread-local slot to close, and the 499s continued exactly as before. Re-examining the actual mechanism: --preload runs `simone.urls` (which starts the singleton Cron thread and, via its first tick, opens a DB connection inside a transaction.atomic() block) in the gunicorn master before workers are forked -- confirmed by the container logs, where "Cron run: starting" always precedes "Booting worker". fork() duplicates whatever that connection has in flight into the worker. With multi- workspace support, essentially every incoming Slack event needs to look up the Workspace row for the team_id, so if the duplicated/corrupted connection leaves Cron's transaction (and any locks it took) stuck open with nothing able to ever commit or roll it back, every subsequent request needing that same table blocks forever waiting on the lock -- nothing logged, until Slack's ~3s client timeout gives up and nginx records a 499. 100% reproducible, matching what's been observed. The robust fix is to stop sharing anything across the fork boundary at all: drop --preload so each worker loads the app (and starts its own Cron thread, opens its own DB connections) fully independently, post-fork, with nothing inherited from a master that did real work first. Verified this reorders app loading to happen after "Booting worker" instead of before, and a DB-touching request still completes in ~30ms. This relies on running a single worker (script/run doesn't pass --workers, so gunicorn defaults to one) so Cron only ever starts once; noted in a comment. Also replaced gunicorn.conf.py's now-defunct post_fork hook with a worker_abort hook that dumps every thread's stack via faulthandler if a worker ever gets SIGABRT'd for timing out, so a future hang is diagnosable from the logs directly instead of another guess-and-redeploy cycle. --- gunicorn.conf.py | 35 ++++++++++++++++++++++++----------- script/run | 2 +- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/gunicorn.conf.py b/gunicorn.conf.py index 5099d46..d3bb7ad 100644 --- a/gunicorn.conf.py +++ b/gunicorn.conf.py @@ -1,14 +1,27 @@ -def post_fork(server, worker): +# We intentionally do NOT run with --preload. `simone.urls` starts a +# singleton Cron background thread (and opens a DB connection) as an import +# side effect (see wsgi.py's `cron.start()`), and --preload would run that +# in the gunicorn master before workers are forked. fork() then duplicates +# whatever the Cron thread has in flight (its DB connection, mid-transaction +# or not) into the worker, which can leave things like table locks stuck +# open with nothing left to ever commit/release them -- every request that +# then needs the same rows hangs forever with nothing logged, until the +# client (Slack, ~3s) gives up. Without --preload, each worker loads the app +# (and starts its own Cron thread) fully independently, post-fork, so there +# is nothing shared to corrupt. +# +# This relies on running a single worker: script/run doesn't pass --workers, +# so gunicorn defaults to one. If that's ever increased, Cron would start +# once per worker and tick (and message Slack) that many times over. + + +def worker_abort(worker): ''' - We run with --preload, so `simone.urls` (and the Cron thread it starts) - is imported in the master process before workers are forked. Any - database connection the master has opened by that point (e.g. Cron's - first tick) gets duplicated into each worker via fork(), leaving the - master and worker sharing one underlying socket. Once both sides use it - concurrently they corrupt each other's protocol state and hang forever - with no error logged. Dropping inherited connections here forces each - worker to lazily open its own on first use instead. + Called when the arbiter SIGABRTs a worker for failing to heartbeat + within --timeout. Dump every thread's stack so a hang shows up in the + logs instead of just a silent restart. ''' - from django.db import connections + import faulthandler + import sys - connections.close_all() + faulthandler.dump_traceback(file=sys.stderr) diff --git a/script/run b/script/run index 709b681..d6e4122 100755 --- a/script/run +++ b/script/run @@ -4,4 +4,4 @@ set -e ./manage.py collectstatic --no-input ./manage.py migrate --no-input -gunicorn simone.wsgi --config gunicorn.conf.py --bind 0.0.0.0:6444 --graceful-timeout 5 --preload --threads 4 +gunicorn simone.wsgi --config gunicorn.conf.py --bind 0.0.0.0:6444 --graceful-timeout 5 --threads 4