Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix case where websocket collision crashes frames #227

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cuttlefisch
Copy link

@cuttlefisch cuttlefisch commented Mar 24, 2022

The Issue

Mentioned in #202, there is a case where successive calls to enable org-roam-ui-mode causes an address collision due to the websocket-server call triggering with each request to activate the mode.

This appears to happen with desktop.el but can also be triggered with the following steps if using DOOM emacs, and emacs daemon.

  1. Create new emacsclient frame: emacsclient -c
  2. Enable org-roam-ui-mode
  3. Change the theme
  4. Launch a new emacsclient frame: emacsclient -c
  5. Observe the new frame crash, and the theme reverts to its previous state.

In this case the behavior is a result of doom's theme-specific hooks resulting in successive calls of (org-roam-ui-mode 1), which cause the websocket-server failure.

Proposed solution

This PR creates a new custom variable to track the state of the webserver: t if running & nil if not. A call to enable the mode only launches the websocket-server if the server is not already running. If the server was already running at the time of the call, log to the Message buffer that the server was already running.

@cuttlefisch
Copy link
Author

Updated the PR to use a function called org-roam-ui-server-runningp in order to provide a reliable check whether the server is listening or running. This replaces the variable used to check startup behavior and removes the need for explicitly modify a variable upon startup/shutdown.

Calls to some of the process-* functions when `org-roam-ui-mode`
is nil results in the target process evaluating to that of the current
buffer. The result is failed server startup/shutdown under certain
common conditions.
@cuttlefisch
Copy link
Author

cuttlefisch commented Apr 25, 2022

This is ready for more thorough testing by others. There was some unexpected behavior which turned out to be a result of the process library's treatment of a nil argument value. The nil arg results in the target process evaluating to that of the current buffer, resulting in some failed startup/shutdown behavior depending on the state of this mode.

The proposed solution is to prevent calls which would act on org-roam-ui-mode-ws-server's process if its value is nil, since the calls would not act on that process anyway. Manual testing seems to indicate this reliably behaves as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] desktop.el and org-roam-ui-mode: Cannot bind server socket: Address already in use
1 participant