Skip to content

Static file server resolves its engine ambiently and starts unserialized #5580

Description

@cjkindel

Problem

Two defects in how the static file server picks up its engine and starts itself. Both are latent while a process holds exactly one Engine, which is the case today, and both become live the moment one does not.

1. Handlers resolve their engine ambiently, on a thread that cannot see it. Every handler in servers/static.py reaches its managers through current_engine() (ten call sites: workspace path, static files manager, library manager). The server runs on a uvicorn thread started by StaticFilesManager.

A new thread starts with an empty contextvars.Context, so _scoped_engine is unset there and current_engine() falls through to the process root engine. A server belonging to a non-root engine would therefore serve the root engine's workspace directory, library widgets and static files: uploads landing in the wrong tree and reads answered from it, silently.

The engine is known at the point the thread is started, so it can be handed over (via app.state for the routes, and to the static-files mount) rather than looked up. Removing the current_engine import from the module is what stops a new handler reintroducing the fallback.

2. _resolve_static_server runs unserialized. Initialization can complete more than once per process, and an unset static_server_base_url is the only thing that stops a later pass starting a second server:

https://github.com/griptape-ai/griptape-nodes-engine/blob/main/src/griptape_nodes/retained_mode/managers/static_files_manager.py#L510

Reading that sentinel is conclusive only while no other pass sits between the read and its own assignment. Two concurrent passes each bind a socket and each start a server thread, on different ports, and the driver keeps whichever wrote last. The first server is then orphaned but still listening.

Impact

None today. Nothing in production constructs a second Engine: engine_scope is called in production only at event_manager.py:693, and only with the manager's own engine. Nothing broadcasts AppInitializationComplete concurrently either.

Both are worth closing because the mechanisms are documented as the guards (the contextvar as the engine binding, the None sentinel as the single-server guard) and neither holds. A future change that adds a second in-process engine, or a second initialization broadcast, gets a wrong-workspace read or a duplicate server rather than an error.

Related

Origin

Found while reviewing the worker PR stack. Not a worker defect and deliberately not fixed there; the branch that carried fixes (#5572) has had them removed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions