On Mon, 18 Mar 2024 23:45:29 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Mon, 18 Mar 2024 23:45:29 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Mon, 18 Mar 2024 23:45:29 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Mon, 18 Mar 2024 11:10:47 GMT, Serguei Spitsyn wrote:
>> Maybe `is_interrupted_with_reset`? (though that would seem like an alias
>> for `is_interrupted(true)`.
>
> Thanks, Alan and David. Will consider your suggestions.
Renamed now.
-
PR Review Comment:
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Mon, 18 Mar 2024 02:21:40 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/javaThread.cpp line 596:
>>
>>> 594: // Checks and clears the interrupt status for platform or virtual
>>> thread.
>>> 595: // Used by the JVMTI RawMonitorWait only.
>>> 596: bool JavaThread::is_interrupted() {
On Mon, 18 Mar 2024 02:23:44 GMT, David Holmes wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: made current changes limitedto just RawMonitorWait
>
> src/hotspot/share/runtime/javaThread.cpp line 595:
>
On Mon, 18 Mar 2024 00:12:55 GMT, Alan Bateman wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: made current changes limitedto just RawMonitorWait
>
> src/hotspot/share/runtime/javaThread.cpp line 596:
>
On Sat, 16 Mar 2024 22:33:49 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Sat, 16 Mar 2024 22:33:49 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Fri, 15 Mar 2024 21:00:16 GMT, Serguei Spitsyn wrote:
>> So the current changes do not limit this to just `RawMonitorWait`. I was
>> expecting to only see additional code in `RawMonitorWait` that emulates what
>> the Java code does to get the virtual and carrier thread interrupt states in
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Fri, 15 Mar 2024 10:05:06 GMT, David Holmes wrote:
>> I've restored the `InterruptedException` catch in the `Object.wait`.
>> However, the fix in `JavaThread::is_interrupted()` also impacts the
>> variations of `sleep_nanos()`.
>
> So the current changes do not limit this to just
On Fri, 15 Mar 2024 09:06:33 GMT, Serguei Spitsyn wrote:
>> Personally I'd prefer to see changes limited to just JVMTI `RawMonitorWait`.
>> That minimises the risk of any unintended consequences from making the
>> change.
>
> I've restored the `InterruptedException` catch in the `Object.wait`.
On Fri, 8 Mar 2024 06:21:17 GMT, David Holmes wrote:
>> It may be that once Object.wait is implemented that we can remove the need
>> to propagate the interrupt status (there are some TBDs here) so things will
>> be a lot simpler.
>>
>> I think the change here is okay for now but we still
On Fri, 15 Mar 2024 09:02:13 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Fri, 8 Mar 2024 07:09:53 GMT, Alan Bateman wrote:
> Can you look at it again? That test has a copy of `Object.java` so needs to
> be updated every time we touch `Object.java`. It's an annoying tax and I hope
> there is a JBS issue to replace that test. In this case, it will be benign
>
On Fri, 8 Mar 2024 05:56:24 GMT, Serguei Spitsyn wrote:
> > Can you check if
> > vmTestbase/nsk/jvmti/scenarios/bcinstr/BI04/bi04t002/TestDescription.java
> > is passing now? That test is a bit annoying in that it fails every time we
> > touch j.l.Object so you might have to update
> >
On Thu, 7 Mar 2024 08:42:11 GMT, Alan Bateman wrote:
>> Use of `ObjectLocker` here will introduce a new pinning point for Loom. We
>> have been removing as many uses of `ObjectLocker` as we can. I also think
>> this will need to be moved back to Java code when the pinning currently
>>
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Thu, 7 Mar 2024 07:28:31 GMT, Alan Bateman wrote:
> Can you check if
> vmTestbase/nsk/jvmti/scenarios/bcinstr/BI04/bi04t002/TestDescription.java is
> passing now? That test is a bit annoying in that it fails every time we touch
> j.l.Object so you might have to update
>
On Thu, 7 Mar 2024 06:45:01 GMT, David Holmes wrote:
>> Fixed as suggested by Alan.
>>
>>> Also need to think through if Object.wait will need to be changed as part
>>> of this.
>>
>> Still need to look at this.
>
> Use of `ObjectLocker` here will introduce a new pinning point for Loom. We
>
On Thu, 7 Mar 2024 00:13:56 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Wed, 6 Mar 2024 10:41:21 GMT, Serguei Spitsyn wrote:
>> Thanks, I'm already working on it.
>
> Fixed as suggested by Alan.
>
>> Also need to think through if Object.wait will need to be changed as part of
>> this.
>
> Still need to look at this.
Use of `ObjectLocker` here will introduce a
On Tue, 5 Mar 2024 23:11:07 GMT, David Holmes wrote:
> Okay so that is where the carrier and virtual thread states get back in sync,
> and that is what is missing in the `RawMonitorWait` case. The proposed
> fix/change to `is_interrupted` is what threw me as that is the wrong place to
> make
On Wed, 6 Mar 2024 18:56:54 GMT, Alan Bateman wrote:
> I think you can drop the catch of InterruptedException in Object.wait now.
> Easy to check this by running
> test/jdk/java/lang/Thread/virtual/MonitorWaitNotify.java as it has tests for
> interrupting Object.wait.
Thanks. Fixed now.
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Wed, 6 Mar 2024 10:44:15 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Wed, 6 Mar 2024 09:34:40 GMT, Serguei Spitsyn wrote:
>> src/hotspot/share/runtime/javaThread.cpp line 573:
>>
>>> 571:
>>> 572: Handle thread_h(current, vthread_or_thread());
>>> 573: ObjectLocker lock(Handle(current,
>>> java_lang_Thread::interrupt_lock(thread_h())), current);
>>
>>
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Wed, 6 Mar 2024 09:27:18 GMT, Alan Bateman wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> removed trailing spaces in javaClasses.cpp and libInterruptRawMonitor.cpp
>
>
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Wed, 6 Mar 2024 09:14:18 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Wed, 6 Mar 2024 06:28:01 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Tue, 5 Mar 2024 19:57:39 GMT, Serguei Spitsyn wrote:
> The behavior of ObjectMonitor::wait and RawMonitorWait is different.
The Object.wait() throws the InterruptedException if it was interrupted. The
RawMonitorWait clears the thread interrupt status and returns the error code
On Tue, 5 Mar 2024 19:57:39 GMT, Serguei Spitsyn wrote:
> The Object.wait() throws the InterruptedException if it was interrupted.
And clears the interrupted status, just like RawMonitorWait.
-
PR Comment: https://git.openjdk.org/jdk/pull/18093#issuecomment-1979670426
On Tue, 5 Mar 2024 07:47:19 GMT, Alan Bateman wrote:
>> Thank you for sharing this, Chris. It sounds like we need to introduce a
>> mechanism to temporarily hide the JVMTI events. The question is if it is
>> worth the complexity we add with it, especially if it is used just in a
>> couple of
On Tue, 5 Mar 2024 07:07:27 GMT, David Holmes wrote:
> I have to say that I don't understand how the behaviour of `RawMonitorWait`
> is any different to `ObjectMonitor::wait` when it comes to the use of the
> is_interrupted(true). ??? Is it simply that because we are in native code and
> we
On Tue, 5 Mar 2024 18:16:03 GMT, Leonid Mesnik wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: addressed a couple of comments on new test
>
>
On Tue, 5 Mar 2024 06:16:04 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
On Tue, 5 Mar 2024 01:06:32 GMT, Serguei Spitsyn wrote:
>>> AFAIK, as it is not a good idea to post the JVMTI events recursively
>>
>> We already do. When the debug agent is handling a JVMTI event and
>> RawMonitorWait is interrupted, that results in the debug agent later on
>> calling
On Tue, 5 Mar 2024 06:16:04 GMT, Serguei Spitsyn wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is that
> when it calls `jt->is_interrupted(true)` to fetch
On Mon, 4 Mar 2024 22:04:39 GMT, Chris Plummer wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue is
On Tue, 5 Mar 2024 00:54:10 GMT, Chris Plummer wrote:
>> @AlanBateman said:
>>> So minimally RawMonitorWait will need to disable suspend and and clear the
>>> interrupt status of both threads while holding the interrupt lock.
>>
>> If we do it with a Java upcall to the
On Mon, 4 Mar 2024 23:35:16 GMT, Serguei Spitsyn wrote:
> AFAIK, as it is not a good idea to post the JVMTI events recursively
We already do. When the debug agent is handling a JVMTI event and
RawMonitorWait is interrupted, that results in the debug agent later on calling
InterrtupThread
On Mon, 4 Mar 2024 11:51:01 GMT, Alan Bateman wrote:
>> Alan and David, thank you for the comments!
>> The initial implementation of the `is_iterrupt()` intentionally avoided any
>> synchronization and allowed a potential raises with the concurrent
>> interrupts. Please, see the comment above
On Fri, 1 Mar 2024 23:26:48 GMT, Serguei Spitsyn wrote:
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is
On Mon, 4 Mar 2024 11:51:01 GMT, Alan Bateman wrote:
>> Alan and David, thank you for the comments!
>> The initial implementation of the `is_iterrupt()` intentionally avoided any
>> synchronization and allowed a potential raises with the concurrent
>> interrupts. Please, see the comment above
On Mon, 4 Mar 2024 09:45:59 GMT, Serguei Spitsyn wrote:
>> On that note, note that the entire comment block preceding this needs to be
>> updated for virtual threads. And it is far from clear to me how to correctly
>> manage the interrupt state of virtual threads within the VM, when it
>>
On Mon, 4 Mar 2024 04:09:35 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/javaThread.cpp line 594:
>>
>>> 592: // thread_oop is virtual, clear carrier thread interrupt status
>>> as well
>>> 593: java_lang_Thread::set_interrupted(threadObj(), false);
>>> 594: }
>>
>>
On Sat, 2 Mar 2024 07:58:40 GMT, Alan Bateman wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue is
On Fri, 1 Mar 2024 23:26:48 GMT, Serguei Spitsyn wrote:
> Please, review this fix correcting the JVMTI `RawMonitorWait()`
> implementation.
> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
> the interrupt status of the interrupted waiting thread. The issue is
Please, review this fix correcting the JVMTI `RawMonitorWait()` implementation.
The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update
the interrupt status of the interrupted waiting thread. The issue is that when
it calls `jt->is_interrupted(true)` to fetch and clear
59 matches
Mail list logo
