JDK-8281652 : com.sun.jdi.ObjectReference::setValue spec should prohibit any final field modification
  • Type: CSR
  • Component: core-svc
  • Sub-Component: debugger
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 20
  • Submitted: 2022-02-11
  • Updated: 2022-11-30
  • Resolved: 2022-11-30
Related Reports
CSR :  
Description
Summary
-------

Change `com.sun.jdi.ObjectReference.setValue` to specify that it cannot be used to change final instance fields, and align it with long standing implementation behavior.

Problem
-------

ObjectReference.setValue spec says:

    Sets the value of a given instance or static field in this object. The Field must be
    valid for this ObjectReference; that is, it must be from the mirrored object's class
    or a superclass of that class. If static, the field must not be final.

The JDK's JDI implementation has never allowed a final field (static or instance) be changed. The current behavior, and spec vs. implementation difference, goes back to 1999.


Solution
--------

Changes the description `ObjectReference.setValue` to specify that the method cannot be used to change final fields.

Change the description of the `IllegalArgumentException` to include the condition that the field is final.


Specification
-------------
old:

    /**
     * Sets the value of a given instance or static field in this object.
     * The {@link Field} must be valid for this ObjectReference; that is,
     * it must be from the mirrored object's class or a superclass of that class.
     * If static, the field must not be final.
     * <p>
     ...
     * @throws java.lang.IllegalArgumentException if the field is not valid for
     * this object's class.
     ...
     */
    void setValue(Field field, Value value)

new:

    /**
     * Sets the value of a given instance or static field in this object.
     * The {@link Field} must be valid for this ObjectReference; that is,
     * it must be from the mirrored object's class or a superclass of that class.
     * The field must not be final.
     * <p>
     ...
     * @throws java.lang.IllegalArgumentException if the field is not valid for
     * this object's class or the field is final.
     ...
     */
    void setValue(Field field, Value value)

Comments
Thanks for the investigation [~alanb]; moving to Approved.
30-11-2022

I've edited the CSR to make it clearer what this issue is about. Initially I had concerns with the proposal but I think it's okay now. To get there has required digging into ancient history and also communication with the main stream IDEs. On the history front, the JDK's JDI implementation has never allowed final fields (instance or static) to be changed. The original intent back in 1999 may have been to allow final instance fields to be changed but the original implementation never allowed it. This means that IDEs using the JDK's JDI (NetBeans, also IntelliJ by way of the JDI fork that they maintain) don't allow it. The Eclipse JDT project have their own implementation of JDI that is used by Eclipse and VS Code. The Eclipse JDT implementation does allow final fields be changed. I contacted the VS Code debugger team in Microsoft and they confirmed that VS Code checks if the field is final and so never calls ObjectReference.setValue on a final field. So not an issue for VS Code. Alex Menkov and Chris Plummer have been in contact with the Eclipse debugger maintainers about this issue and Eclipse seem to be okay with the change (see PR comments). I don't know if there is an issue in the Eclipse bugzilla yet on this issue. It's possible that there are other tools in the eco system using the Eclipse JDI implementation and relying on it to change final fields but we have no information on that.
30-11-2022

Moving to Provisional, not Approved.
30-11-2022

Moving back to draft. [~amenkov], please have one or more engineers review this CSR before Proposing or Finalizing it.
22-11-2022

I see this CSR has been finalized but this one will require more investigation before a CSR can be written.
22-11-2022

The "Compatibility Risk Description" field needs to filled in. Other than that, it looks good.
22-11-2022