On Tue, 23 Jun 2026 11:08:44 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to improve the 
>> exception message in `IllegalMonitorStateException` when it gets thrown for 
>> value objects?
>> 
>> With this change, we now report the following exception message when value 
>> objects are used to invoke `Object.wait()` and related methods:
>> 
>> 
>> jshell> o.wait()
>> |  Exception java.lang.IllegalMonitorStateException: current thread is not 
>> owner
>> |        at Object.wait0 (Native Method)
>> |        at Object.wait (Object.java:433)
>> |        at Object.wait (Object.java:387)
>> |        at (#2:1)
>> 
>> There were some thoughts about using an exception message of the form 
>> `Exception java.lang.IllegalMonitorStateException: object's type 
>> java.lang.Integer is not an identity class`. But since value objects cannot 
>> be synchronized upon, we decided it would be fine (and accurate) to reuse 
>> the same "current thread is not owner" exception message that we use for 
>> identity objects.
>> 
>> Given the nature of this change, I haven't added any new jtreg tests for 
>> this. Existing tests in tier1, tier2 and tier3 continue to pass after this 
>> change.
>> 
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - merge latest from lworld branch
>  - no need for ResourceMark
>  - 8386963: [lworld] Improve the exception message from Object 
> synchronization methods on value objects

There's been some discussion on whether to throw an `IdentityException` instead 
of `IllegalMonitorStateException` from these `Object.wait()/notify/notifyAll` 
methods when invoked on value objects. There hasn't been any consensus yet and 
for now it seems that we will continue with the `IllegalMonitorStateException` 
being thrown.

I don't have any more changes planned for this PR. The change here will merely 
improve the exception message contained in the `IllegalMonitorStateException`. 
I think that part is trivial and several of us agree that the new message is 
more appropriate.

The other change in this PR is the removal of `ResourceMark` from these if 
blocks. I don't have prior knowledge of that construct, so I would like a 
review from Alan and/or someone who knows more about this so that we can have 
this change integrated into `lworld` in preparation for the JEP 401 integration.

-------------

PR Comment: https://git.openjdk.org/valhalla/pull/2568#issuecomment-4796499964

Reply via email to