RFR: 8355960: JvmtiAgentList::Iterator dtor double free with -fno-elide-constructors

[David Holmes]

On Wed, 2 Jul 2025 01:47:59 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

> Currently jvmtiAgentList keeps agents in reversed order (new agents are added to the head of the list).
> To restore original order JvmtiAgentList::Iterator uses GrowableArray allocated in heap.
> Iterators for different agent types are returned by value, and the iterator class nas no custom copy ctor, so if the constructor not elides, GrowableArray is deallocated twice.
> 
> The fix updates jvmtiAgentList to keep agents in the original order, agents are added to the tail.
> Iterator now needs only single pointer to next agent.
> Additionally removed `JvmtiAgentList::Iterator::next() const` method (it looks very strange as `next()` is expected to change state of the iterator).
> 
> Testing: tier1..4,hs-tier5-svc

I'm not very familiar with these aspects of C++ but this seems to be a very complex solution to what sounds in JBS to be a pretty straight-forward problem.

src/hotspot/share/prims/jvmtiAgentList.cpp line 35:

> 33: 
> 34: JvmtiAgent* JvmtiAgentList::_head = nullptr;
> 35: JvmtiAgent** JvmtiAgentList::_tail = nullptr;

Why is the tail a pointer to a pointer? Sorry I'm not understanding how your list is being managed here.

I'm trying to work out where you need acquire/release because I'm pretty sure it is missing where needed, but again I can't understand how you are actually constructing and using the list.

src/hotspot/share/prims/jvmtiAgentList.cpp line 105:

> 103:   while (true) {
> 104:     // set _tail to address of agent->_next
> 105:     JvmtiAgent** tail = Atomic::load_acquire(&_tail);

You don't need acquire semantics here as you are not doing anything with the `_tail` pointer value other than use it in relation to cmpxchg which provides fully memory barriers.

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

PR Review: https://git.openjdk.org/jdk/pull/26083#pullrequestreview-2977985314
PR Review Comment: https://git.openjdk.org/jdk/pull/26083#discussion_r2179305538
PR Review Comment: https://git.openjdk.org/jdk/pull/26083#discussion_r2179284415