Skip to content

Revisit server startup sequence #1028

@alcarney

Description

@alcarney

To quote a recent commit message

lsp: Run manager setup during initialize

This batch of changes has unearthed a bunch of interesting timing issues
during server start up.

I was seeing an issue where the esbonio.sphinx.pythonCommand
configuration was not being applied only in the case where I had one
or more *.rst files open before restarting the server.

I eventually realised that this was due to the textDocument/didOpen
notification handlers winning the race with the initialized
notification handler and creating a SphinxConfig configuration
subscription before the additional structure hooks in the
SphinxManager could be registered.

This meant the promotion of list[str] values to SubProcess values
was not happening for these early subscriptions.

More broadly, this probably points to a need to reconsider the startup
sequence at some point, but running this code during initialize
(during which time other messages are forbidden) should be enough to
paper over the issue for now.

And another

lsp: Delay server ready until after features are initialized

Related to the previous commit, the server was declaring itself ready
before all the initialized handlers of the features were processed.

This commit therefore attempts to be more correct by delaying the
resolution of the ready future until after the execution of these
handlers. However, the previous commit also demonstrates how this
already insufficient to ensure correct behaviour as other
requests/notifications are free to be serviced concurrently with
initialized.

The "obvious" fix for this would be to move all setup code into the
initialize handler, but there is at least one situation where this
cannot be done and that is the configuration system. Initializing the
configuration system involves making workspace/configuration requests
to the client - which are forbidden until after the initialize result
has been sent!

As I write this, perhaps the other "obvious" solution is to have all of
the message handlers in esbonio/server/setup.py consult the state of
the ready future before proceeding... but for some reason (I can't quite
articulate why) that doesn't feel like an ideal solution...

And there's other timing issues like the logging system not waking up until long after all the sphinx clients have been setup etc.

I don't think this is worth blocking the 1.0 release on, but the sooner something can be figured out the better...

Metadata

Metadata

Assignees

No one assigned

    Labels

    lspIssues that relate to the language server

    Type

    No type

    Projects

    Status

    Todo

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions