DatabaseSessionService.append_event raises MissingGreenlet with asyncpg after client disconnect (regression from da73e718ef) #5761

Activity

1. surajksharma07

   self-assigned this

   on May 20, 2026
2. surajksharma07

   added the

   Bug

   issue type on May 20, 2026
3. surajksharma07

   added

   services[Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc[Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

   on May 20, 2026
4. KrishnaVemu commented on May 20, 2026

   

   KrishnaVemu

   on May 20, 2026

   Author

   More actions

   Thanks for digging in — but I think the cancellation framing is leading us away from the actual mechanism. Let me lay out what I've verified, because the specifics matter for the fix.

   1. The greenlet bridge is per-await, not per-AsyncSession context.

   Each await async_session.something() spawns a fresh greenlet via greenlet_spawn, runs the sync ORM code inside it, and the greenlet exits when that await returns. It does not persist across awaits, and async with AsyncSession(...) is not what creates it. So "the surrounding task gets cancelled and the next await_only() lands without its greenlet" mischaracterizes the bridge. Cancellation between awaits would just give a normal CancelledError, not MissingGreenlet.

   2. da73e718ef did not move the commit or swap explicit awaits for implicit ones.

   The diff is one line: engine_kwargs.setdefault("pool_pre_ping", True) for non-SQLite engines. That's it. Verifiable in database_session_service.py:208. The commit shape is neither (a) nor (b) you proposed.

   3. The actual mechanism is pool_pre_ping + onupdate=func.now() + sync attribute access.

   schemas/v1.py:96 declares update_time with onupdate=func.now(). After the UPDATE flush, SQLAlchemy doesn't know what timestamp Postgres wrote, so it marks the attribute as expired — independent of expire_on_commit. Then database_session_service.py:743 reads it:

   await sql_session.commit()                                                   # L740
   session.last_update_time = storage_session.get_update_timestamp(is_sqlite)   # L743

   The read on L743 happens via __get__ on a Mapped attribute — plain sync attribute access, no await — which triggers _load_expired → load_on_pk_identity → sync pool checkout → pool_pre_ping=True → asyncpg dialect's do_ping → await_only(self._async_ping()) → no surrounding greenlet → MissingGreenlet. The full chain is in the trace at the top of this issue.

   Client disconnect is not required — it just changes the pool's checkout timing so the bug surfaces reliably. Without disconnect, the same connection used for the commit is sometimes still hot in the fairy and pre-ping is skipped; with disconnect, the teardown returns it and the next checkout pings.

   4. Why the proposed fixes don't address it:

   • asyncio.shield(session.commit()) — the commit is not what fails. L743 fires after a successful commit. Shielding the commit changes nothing.
   • except asyncio.CancelledError around append_event — there's no CancelledError in this trace, only MissingGreenlet. The bug reproduces without any cancellation.

   5. Cleanest structural fix is at L737–L743, not at cancellation boundaries.

   Either:

   (a) Read into a local before commit so no lazy-load is needed:

   storage_session.update_time = update_time
   await sql_session.flush()
   await sql_session.refresh(storage_session, ["update_time"])  # one await, inside the bridge
   updated_at = storage_session.update_time
   await sql_session.commit()
   session.last_update_time = (
       updated_at.replace(tzinfo=timezone.utc).timestamp() if is_sqlite
       else updated_at.timestamp()
   )

   (b) Drop onupdate=func.now() and trust the Python-assigned value (it's already set on L737), which removes the post-commit refresh entirely.

   Both make the bug structurally impossible regardless of pool_pre_ping or cancellation timing. A regression test should just hit the endpoint normally — no client cancellation choreography needed.
5. surajksharma07 commented on May 20, 2026

   

   surajksharma07

   on May 20, 2026

   Collaborator

   More actions

   @KrishnaVemu the pool_pre_ping default introduced in da73e71 combined with the expired update_time on the post-commit attribute access is a precise root cause.

   Worth trying: move both get_update_timestamp(is_sqlite) and get_update_marker() into locals before the await sql_session.commit() at L747, then assign them to session after.
   That removes any post-commit ORM attribute access entirely and sidesteps the lazy-load/greenlet bridge issue.

   Could you test this thoroughly in your environment and confirm it consistently clears the MissingGreenlet across your use cases.
6. surajksharma07

   added

   request clarification[Status] The maintainer need clarification or more information from the author[Status] The maintainer need clarification or more information from the author

   on May 20, 2026
7. kkj333

   added a commit that references this issue on May 28, 2026

   fix(sessions): Prevent MissingGreenlet after append_event with asyncpg

   ...

   This issue will close once commit 08ec380 is merged into the 'main' branch.

   08ec380
8. DeanChensj commented on Jun 17, 2026

   

   DeanChensj

   on Jun 17, 2026

   Collaborator

   More actions

   Fixed by #5814
9. DeanChensj

   closed this as completedon Jun 17, 2026