-
Notifications
You must be signed in to change notification settings - Fork 133
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
clarify object lifetime in evaluate/output #462
Conversation
I struggle to see how this clarification really clarifies anything. Keeping them alive as long as possible isn't really something client authors can rely on. I'm actually more confused now than before. I always assumed that evaluate requests had to be re-sent on every stopped event and values were otherwise invalid. Now it seems they might still be valid? Hard to know what to do with that info tbh. |
It seems to me that this particular use case really needs explicit lifetime management in form of some kind of "releaseVariableReference" request that client can issue to indicate that it's done with it. Otherwise, there's just no sensible way to use it, since the guarantee here is weak enough that in practice the variable may well be gone by the time "output" even is received and processed. This would actually be good to have for other reasons. In the Python debugger, as an extension, we provide the ability to issue "evaluate" requests (without "frameId") even while debugger is running - this turned out to be useful for things that build on top of the debugger, such as Jupyter notebooks, as a generic facility to run some code inside the debuggee to retrieve values or for side effects. But, due to lifetime issues around variable references, we had to restrict this feature to returning stringified values only. Allowing manual lifetime control would allow for structured data, making it so much more flexible. Also note that there is a distinction here between lifetime of the reference (i.e. variable ID), and lifetime of the actual variable itself. Depending on the runtime, lifetime of the variable might not be something that debugger can even control. In such cases, I think it's fine for "variables" request to return an error; the important thing is that variable ID is guaranteed to refer to the same variable throughout, and not be reused for another unrelated variable until the client explicitly releases it. This kind of race condition seems to be allowed by the present wording of the spec, or at least, I cannot find any verbiage that prohibits debug adapters from reusing ID once variable lifetime ends for whatever reason. |
It's valid to send
That sounds like a good idea. However it might be difficult at this point for debug adapter authors to depend on, since they don't know if a consumer will send a "release" request/event. While we could hint it in Something that the Chrome Devtools protocol does is allowing the client to specify an What do you think?
Yes, that's a good distinction to make -- it also relates to "Keeping them alive as long as possible isn't really something client authors can rely on." This has always been the case, and if the user tries to interact with a variable and that request results in an error, the client should display an error. One easy case in VS Code is logging an object and ending the debug session. Attempting to then expand the object properties shows an error. |
The problem with client-initiated lifetime management protocol is that it inherently doesn't work with cases where the adapter generates a variable ID out of the blue (as is the case with "output" events with variables). On the other hand, requiring explicit lifetime management for existing parts of DAP would be a breaking change, so it's not really an option here, either. I don't see any way out of this conundrum short of adding a new, distinct property to "output" for the DA specifically for variables that require such lifetime management (and the corresponding capability etc, so that such messages are only sent to clients that are aware of the requirement). But for "evaluate" requests, having some kind of property that would tell the adapter that the resulting variable reference should never be auto-disposed seems like a sensible approach to me. And yes, I think it would help a lot if all IDs were non-reusable. Is it viable, though, given that the spec limits IDs to 32-bit ints in practice? It feels like a typical debug session, with user manually triggering actions that cause IDs to be generated, would be extremely unlikely to exhaust 2**32 IDs for any kind of DAP entity. But the client needs not be controlled by a user; scripted DAP use is also a thing. |
Closes #459