PING: Re: RFR: 8219904: ClassCastException when calling FlightRecorderMXBean#getRecordings()

[Erik Gahlin]

Hi,

If you can test this without connecting over the network it would be good.

Not sure if it  helps, but it is possible to attach to the same process 
if you set the system property at startup, -Djdk.attach.allowAttachSelf=true

Could you also check if the property "toDisk" is missing, and if so 
check if "disk" is set, and if so use it. This is in the case somebody 
called the RecordingInfo:: from with "disk" (as the javadoc stated 
previously). Not sure why anyone would do so, but that way we will 
prevent code from breaking.

Thanks
Erik

On 2019-11-26 15:45, Chihiro Ito wrote:
> Hi,
>
> Could you please review this?
>
> Regards,
> Chihiro
>
> 2019年11月19日(火) 0:54 Chihiro Ito <chiroito107 at gmail.com 
> <mailto:chiroito107 at gmail.com>>:
>
>     Hi Erik,
>
>     I fixed the implementation. Could you review this, please?
>
>     After your review, may I copy the diff of RecordingInfo class into
>     the specification section?
>     This is my first time to do CSR, so I'm sorry if I made a mistake
>     in the procedure.
>
>     Webrev : http://cr.openjdk.java.net/~cito/JDK-8219904/webrev.02/
>     JBS : https://bugs.openjdk.java.net/browse/JDK-8219904
>     CSR : https://bugs.openjdk.java.net/browse/JDK-8234305
>
>     Regards,
>     Chihiro
>
>
>     2019年11月18日(月) 9:24 Erik Gahlin <erik.gahlin at oracle.com
>     <mailto:erik.gahlin at oracle.com>>:
>
>         Hi Chihiro,
>
>         I created a CSR draft:
>
>         https://bugs.openjdk.java.net/browse/JDK-8234305
>
>         If you fix the implementation, we can then copy the changes to
>         the RecordingInfo class (in a diff format) in the
>         specification section.
>
>         Thanks
>         Erik
>
>
>>         On 17 Nov 2019, at 16:14, Chihiro Ito <chiroito107 at gmail.com
>>         <mailto:chiroito107 at gmail.com>> wrote:
>>
>>         Hi Erik,
>>
>>         I also think It is better.
>>         I would like you to help me submit CSR.
>>
>>         Regards,
>>         Chihiro
>>
>>         2019年11月14日(木) 2:46 Erik Gahlin <erik.gahlin at oracle.com
>>         <mailto:erik.gahlin at oracle.com>>:
>>
>>             Hi Chihiro,
>>
>>             I think it would be better to change the javadoc from
>>             "disk" to "toDisk" and not add a new method the
>>             RecordingInfo class as this is less of an intrusive
>>             change, to JMC and others.
>>
>>             Still, a CSR needs to be filed, as this impacts
>>             compatibility. Do you want help with this?
>>
>>             The constructor could support both disk and toDisk,
>>             similar to what you have today.
>>
>>             Thanks
>>             Erik
>>
>>             On 2019-11-13 14:12, Chihiro Ito wrote:
>>>             Hi Erik,
>>>
>>>             Could you please review this?
>>>
>>>             Regards,
>>>             Chihiro
>>>
>>>             2019年10月25日(金) 0:43 Chihiro Ito <chiroito107 at gmail.com
>>>             <mailto:chiroito107 at gmail.com>>:
>>>
>>>                 Hi Erik,
>>>
>>>                 I leave the isToDisk method to maintain
>>>                 compatibility so that I can connect to Java
>>>                 processes that this issue does not resolve.
>>>                 The isToDisk method was deprecated.
>>>
>>>                 Could you review this, please?
>>>
>>>                 JBS https://bugs.openjdk.java.net/browse/JDK-8219904
>>>                 webrev
>>>                 http://cr.openjdk.java.net/~cito/JDK-8219904/webrev.01/
>>>
>>>                 Regards,
>>>                 Chihiro
>>>
>>>                 2019年10月24日(木) 1:12 Chihiro Ito
>>>                 <chiroito107 at gmail.com <mailto:chiroito107 at gmail.com>>:
>>>
>>>                     Hi Erik,
>>>
>>>                     Thank you for prompt reply.
>>>
>>>                     I also checked the Javadoc. The key should be disk.
>>>
>>>                     > Is "toDisk" synthesized from the method name
>>>                     "isToDisk" in
>>>                     > RecordingInfo, or is it set somewhere else?
>>>
>>>                     Yes, it appears to be composited from the
>>>                     isToDisk method.
>>>                     For compatibility, I think you should leave the
>>>                     isToDisk method and add @Deprecate, what do you
>>>                     think?
>>>
>>>                     This problem occurs when attaching over a
>>>                     network or to a process. The process is
>>>                     connected over the network because it cannot be
>>>                     attached to its own process.
>>>
>>>                     Regards,
>>>                     Chihiro
>>>
>>>                     2019年10月21日(月) 21:13 Erik Gahlin
>>>                     <erik.gahlin at oracle.com
>>>                     <mailto:erik.gahlin at oracle.com>>:
>>>
>>>                         Hi,
>>>
>>>                         The javadoc states that the key should be
>>>                         "disk" (which is the same used
>>>                         by jcmd and -XX:StartFlightRecording).
>>>
>>>                         Is "toDisk" synthesized from the method name
>>>                         "isToDisk" in
>>>                         RecordingInfo, or is it set somewhere else?
>>>
>>>                         Do the problem only occur if you access
>>>                         RecordingInfo over network? I am
>>>                         asking since you added a port to the test.
>>>
>>>                         Erik
>>>                         > Hi
>>>                         >
>>>                         > I fixed a problem with ClassCastException.
>>>                         Also, I fixed the CompositeData
>>>                         > field name because it was incorrect.
>>>                         >
>>>                         > Could you please review it?
>>>                         >
>>>                         > JBS
>>>                         https://bugs.openjdk.java.net/browse/JDK-8219904
>>>                         > Webrev
>>>                         http://cr.openjdk.java.net/~cito/JDK-8219904/webrev.00/
>>>                         >
>>>                         > The comment on the JBS mentions
>>>                         UneclaredThrowableException, which is a
>>>                         > different issue.
>>>                         >
>>>                         > Regards,
>>>                         > Chihiro
>>>
>