Skip to content

Interleaving textDocument/codeAction and codeAction/resolve with another type of request gives a wrong response #3135

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

Open
HaraldKi opened this issue Apr 16, 2024 · 3 comments

Comments

@HaraldKi
Copy link

Platform:

  • emacs/eglot 1.17 (but should be irrelevant, see protocol log)
  • eclipse.jdt.ls 1.35.0-202404090315 (also happens with 1.33.0 and Java 21)
  • Java 22
  • gradle project configured for Java 22

Problem summary with time stamps from the attached log:

  • 19:14:47.479 client sends textDocument/codeAction
  • 19:14:47.647 server responds with textDocument/codeAction
  • user ponders which action to take
  • 19:14:48.479 meanwhile the client sends some other request (textDocument/formatting in the log attached)
  • 19:14:48.515 server answers this request
  • user decides on code action (remove unused variable)
  • 19:14:49.312 client sends codeAction/resolve with the CodeAction copied 1:1 as received above, according to the LSP protocol afaict.
  • 19:14:49.313 server responds codeAction/resolve but not with the action to take, rather it returns exactly the 1:1 copy again to the client
  • 19:14:49.313 client interprets the response as something that needs to be resolve and sends the same resolve request again
  • 19:14:49.313 sends the same response

In the case of Eglot, the last two steps repeat until the stack is full and it terminates with an error.

Additional information:

  • I kept all intermediate request/response logging as is in the attached log in case it is also relevant.
  • The problem only happens if the formatting request has a non-empty result. This makes it easy to switch between wrong and correct results.
  • Whether the code action is within the requested formatting range or before makes no difference.
  • The problem also happens if instead of formatting the client requests semantic tokens

Attached request/response log:
remove-xxx-jrpc-format.log

For reference, here is the code I use in my emacs init file to trigger this easily with F8 using eglot, though I don't see anything eglot-specific in the above sequence of events.

(defun boom (buffer)
  (with-current-buffer buffer 
    (eglot-format nil nil nil) ) )
(defun boom-boom ()
  (interactive)
  (run-with-timer 1 nil (apply-partially 'boom (current-buffer)))
  (call-interactively 'eglot-code-actions)
  )
(define-key eglot-mode-map (kbd "<f8>") 'boom-boom)
@rgrunber
Copy link
Contributor

rgrunber commented Oct 4, 2024

Thanks for filing. Is it still happening ? I'd try with v1.37.0 or newer, since there might be some relation to #3180. It uncovered some issues in how we store code actions meant to be resolved.

However, I assume an easy workaround for this issue would be to check if the edit field actually got populated for the code action ? Obviously if it comes back undefined, then it wasn't resolved. It doesn't solve the real problem but would prevent a stack overflow.

It's definitely weird for the edit to not be populated, but even stranger is that you're getting the data field included in the server's response. If you look at https://github.com/eclipse-jdtls/eclipse.jdt.ls/blob/v1.35.0/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java#L33-L38 you'll see the first thing we do is clear out the data field, so even if we somehow failed to resolve the edit, I'd expect the data field to be empty.

@HaraldKi
Copy link
Author

HaraldKi commented Oct 8, 2024

I am busy with other things, but definitively want to check this again with the newer version, hopefully next week.

@HaraldKi
Copy link
Author

The emacs snippet invoking textDocument/codeAction and codeAction/resolve interleaved still goes boom with 1.43.0, this time somewhat differently in sending emacs lisp into a bottomless recursion.

The log file contains the exchange between emacs and the server version 1.43.0
jdtls-3135-with-1_43.log

The course of events in overview:

  • 12:26:50.341: request textDocument/codeAction
  • 12:26:50.356: response textDocument/codeAction
  • ... user pondering code action options ...
  • 12:26:51.341: formatting request send
  • 12:26:51.349: formatting response
  • ... progress and log messages
  • ... user still pondering code action options, then deciding on Organize Imports.
  • 12:26:54.549: request codeAction/resolve
  • 12:26:54.549: response codeAction/resolve
  • ... more such pairs until stack overflow (clipped in log)

So far I have no idea if the server's response to codeAction/resolve makes sense or not, but a quick look at the LSP spec makes me wonder if it should have an edit field, but it is only:

{
  "jsonrpc": "2.0",
  "id": 272,
  "result": {
    "title": "Organize imports",
    "kind": "source.organizeImports",
    "diagnostics": [],
    "data": {
      "pid": "0",
      "rid": "2"
    }
  }
}

No doubt the edit field is optional and Eglot maybe should handle it differently, but if I had a clear indication that the response above is legit I could make a case in an Eglot issue.

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

No branches or pull requests

2 participants