Re: [RFA/JDWP] VirtualMachineCommandSet.java

[Bryce McKinlay <mckinlay at redhat dot com>]

Aaron Luchko wrote:

> Ok this is a touch more complicated but not too bad, there's also a
> couple possibly minor issues. This implements the VirtualMachine
> CommandSet
> http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_VirtualMachine
>
> The VERSION command is the first one that is a bit of an interpretation
> http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_VirtualMachine_Version
> All the fields are pretty direct except for the first one.
> "description: Text information on the VM version"
> Sun along with IBM and BEA (they all actually return the exact same
> response for this command) return
> "Java Debug Wire Protocol (Reference Implementation) version 1.4?JVM
> Debug Interface version 1.3?JVM version 1.4.2_06 (Java HotSpot(TM)
> Client VM' mixed mode)"
>  
IBM and BEA claim to be HotSpot(TM)? That is odd - sounds like a bug in 
Sun's JDWP implementation if so.

> Which seems pretty generic/non-standard so I just did 
>
> Properties props = System.getProperties();
>
> // The description field is pretty loosely defined
> String description = "JVM version " + props.getProperty("java.vm.name")
>                     + " " + props.getProperty("java.vm.version") + " "
>                     + props.getProperty("java.version");
>
> which for my rpm installed gcj gave,
> "JVM version GNU libgcj 4.0.0 20050622 (Red Hat 4.0.0-13) 1.4.2"
>  

Looks good, but should the JDWP version be incorporated into this string 
as well?

> The next issue, the commands 
> AllThreads and TopLevelThreadGroups
> http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_VirtualMachine_Version
>
> Rely on two things:
> 1. There can only be 1 top level thread group.
> 2. The aforesaid top level group can be attained via recursively going
> group.getParent().
>
> If those assumptions are incorrect then things could get more
> complicated.
>  

It is correct that there can only be 1 top level threadgroup for user 
code (since a ThreadGroup cannot be created without a parent), but 
perhaps VMs could implement hidden threadgroups for system code? The 
name of the command, "TopLevelThreadGroups" seems to suggest this.

> There's also the theoretical concern if these are true of a user run
> program being able to find our Jdwp threads via the same way I do
> executeAllTheads().
>  

Access to the thread lists is a secure operation - untrusted code will 
not be able to access these threads if the AccessController's security 
policy doesn't allow it. Likewise, it would be very silly for an 
application to rely on only certain threads existing, VMs make threads 
to do things "behind the back" of the application all the time.

> release/holdEvents
> http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_VirtualMachine_HoldEvents
> should be fine being left on the back burner until later.
>  
OK. It looks like to implement this, you'll need to be able to queue 
outgoing packets after all.

> 2005-07-12  Aaron Luchko  <aluchko@redhat.com>
>
>        * gnu/classpath/jdwp/processor/VirtualMachineCommandSet.java:
>        New file.
>  

You could add a brief description of what the file does to the ChangeLog 
entry, if you like.

> +    // The description field is pretty loosely defined
> +    String description = "JVM version " + props.getProperty("java.vm.name")
> +                         + " " + props.getProperty("java.vm.version") + " "
> +                         + props.getProperty("java.version");
> +    int jdwpMajor = 1; // Get from jvm?
> +    int jdwpMinor = 5; // Get from jvm?
>  
Should this version be 1.4? Also, since this is the implementation, the 
JDWP code itself probably know better than anywhere else what JDWP 
version is implemented.

> +    String vmVersion = props.getProperty("java.version"); // Get from jvm?
>  
The java.version property comes from the VM, at least in libgcj, so it 
should be considered the canonical source.

> +  private void executeClassesBySignature(ByteBuffer bb, DataOutputStream os)
> +    throws JdwpException, IOException
> +  {
> +    String sig = JdwpString.readString(bb);
> +
> +    ArrayList allLoadedClasses, allMatchingClasses;
> +
> +    // This will be a vector of all loaded Classes
> +    allLoadedClasses = vm.getAllLoadedClasses();
>  

This is another example of something that could probably be implemented 
a lot more efficiently in the VM rather than in Java. The problem is 
that the VM will have to enumerate the list of all loaded classes into 
an array/ArrayList/etc, which means quite a bit of memory usage and copying.

If it must be done in Java then it might make sense to have the vm 
interface return an Iterator rather than a List or ArrayList. This way, 
a VM which maintains its own internal class lists anyway could implement 
an iterator around them instead of having to make a copy.


> +  private void executeDispose(ByteBuffer bb, DataOutputStream os)
> +    throws JdwpException
> +  {
> +    // TODO: resumeAllThreads isn't sufficient as a thread may have been
> +    // suspended multiple times, we likely need a way to keep track of how many
> +    // times a thread has been suspended or else a stronger resume method for
> +    // this purpose
> +    vm.resumeAllThreadsExcept(jdwp.getJdwpThreadGroup());
> +
> +    // Simply shutting down the jdwp layer will take care of the rest of the
> +    // shutdown other than disabling debugging in the VM
> +    vm.disableDebugging();
> +  }
> +
> +  private void executeIDsizes(ByteBuffer bb, DataOutputStream os)
> +    throws JdwpException, IOException
> +  {
> +    ObjectId oid = new ObjectId();
> +    os.writeInt(oid.size()); // fieldId
> +    os.writeInt(oid.size()); // methodId
> +    os.writeInt(oid.size()); // objectId
> +    os.writeInt(new ReferenceTypeId((byte) 0x00).size()); // referenceTypeId
> +    os.writeInt(oid.size()); // frameId
> +  }
> +
> +  private void executeSuspend(ByteBuffer bb, DataOutputStream os)
> +    throws JdwpException
> +  {
> +    vm.suspendAllThreadsExcept(jdwp.getJdwpThreadGroup());
> +  }
>  

Unfortunately, I think its going to be hard to make the JDWP code 
completely robust in the face of possible deadlocks caused by user code 
holding locks while suspended. For example, suppose that a user thread 
is in the middle of initializing some class that is also (directly or 
indirectly) used by JDWP. That thread will hold a lock on that class, 
and the JDWP will deadlock. We already try to minimize the number of 
user-visible locks used by libgcj in order to try and avoid the 
possibility of this, but when you allow thread suspension it becomes 
much more difficult. Whether this will be a real problem that is 
encountered in practice, I'm not sure.

Again, these are just a few things to think about. Patch is OK to commit 
with the version ID and ChangeLog changes I mentioned.

Bryce