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

[Chihiro Ito]

Hi Erik,

Thank you for reviewing and advice.
I had referred to other CSR,  I appended the source code and the webrev
link to the specification in the CSR.

Could you review the CSR, please?

CSR : https://bugs.openjdk.java.net/browse/JDK-8234305

Regards,
Chihiro

2019年11月29日(金) 0:46 Erik Gahlin <erik.gahlin at oracle.com>:

> Hi Chihiro,
>
> Looks good.
>
> Please add spec. change to the CSR and I will review it. You can then
> finalize the CSR and then it can take up to a week for a reply.
>
> If it is accepted, I can then sponsor your fix.
>
> Thanks
> Erik
> On 2019-11-28 16:36, Chihiro Ito wrote:
>
> Hi Erik,
>
> Thank you for your review.
>
> I fixed the connection method to use attach and checked the property
> "toDisk" is existing.
> Could you please review this again?
>
> Webrev : http://cr.openjdk.java.net/~cito/JDK-8219904/webrev.03/
>
> Regards,
> Chihiro
>
>
> 2019年11月27日(水) 0:07 Erik Gahlin <erik.gahlin at oracle.com>:
>
>> 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>:
>>
>>> 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>:
>>>
>>>> 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> 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>:
>>>>
>>>>> 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>:
>>>>>
>>>>>> 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>:
>>>>>>
>>>>>>> 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>:
>>>>>>>
>>>>>>>> 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
>>>>>>>>
>>>>>>>>
>>>>