search

castwide / readapt

A Ruby debugger for the Debug Adapter Protocol.
MIT License
42 stars 3 forks source link

Thread id: 64 vs 32 bits? #6

Closed PyvesB closed 4 years ago

PyvesB commented 4 years ago

Hi @castwide!

When responding to a Threads request, Readapt returns an id that may not fit within a 32-bit representation. On my macOS machine, I've for example seen a value of 70268739693720.

The Debug Adapter Specification states that this value should be of type number. However, this is incorrect, the actual specification source indicates integer. This discrepancy between the website and the source is tracked by this issue.

Unfortunately, the specification does not unambiguously state what an integer is in the context of the protocol, but many people seem to be leaning towards 32-bit values (see https://github.com/microsoft/debug-adapter-protocol/pull/63 and https://github.com/microsoft/debug-adapter-protocol/issues/90 as examples).

As far as implementations of the specification are concerned:

I found that the Threads response was built here: https://github.com/castwide/readapt/blob/05df605c1c2b497763bba70761ca7f36fd1cd55c/lib/readapt/message/threads.rb#L10

Should the value be scaled down to a 32-bit value at this point? What are your thoughts?

castwide commented 4 years ago

That's a tricky one. I haven't run into it yet. It might be platform-specific, or I may have just been lucky.

The challenge is that Readapt uses Thread#object_id for the id sent to the client, and Ruby internally stores object_id as long. I'm not sure how to convert them to 32-bit without creating potential for collisions from overflows. Example: on my machine, the object_id for nil is 8.

This problem is also likely to manifest elsewhere, since Readapt uses object_id to identify frame bindings and variables.

No idea how to solve it yet, but you got me thinking. I'm open to suggestions.

PyvesB commented 4 years ago

Maybe one approach could be to drop object_id and use a global counter to assign numerical identifiers to the different objects Readapt is keeping track of. If the counter is started at 0 and is systematically increased for each new assignment, there shouldn't be any risk of overflow or collision.

castwide commented 4 years ago

That could work. The debugger's reference counter should be significantly lower than Ruby's internal ObjectSpace count. We might even be able to reset it between breakpoints.

I'll start a branch for it.

castwide commented 4 years ago

Gem version 1.1.0 uses 32-bit integers to identify threads, variables, and bindings.

PyvesB commented 4 years ago

Thanks for the update @castwide, Things seem to work fine on Windows, but I'm having trouble running the debugger on my macOS machine. However, I'm unsure whether it's anything related to this issue. Will need some more time to set a proper dev environment again and investigate in more depth. Will come back to you in the coming days or weeks!

PyvesB commented 4 years ago

I've started over with a clean setup on my macOS and seems now seem to work as expected. Thanks, closing this issue! 👍🏻