The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: James Clark <james.clark@linaro.org>
To: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
Cc: acme@kernel.org, adrian.hunter@intel.com,
	alexander.shishkin@linux.intel.com, irogers@google.com,
	jolsa@kernel.org, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, mark.rutland@arm.com,
	mingo@redhat.com, namhyung@kernel.org, peterz@infradead.org,
	yuzhuo@google.com
Subject: Re: [PATCH] perf jit: close agent in Agent_OnLoad()
Date: Wed, 24 Dec 2025 11:48:16 +0000	[thread overview]
Message-ID: <ef9af950-84d7-408c-8f1d-e9e75c6dc561@linaro.org> (raw)
In-Reply-To: <20251224113918.1593967-1-lihaoxiang@isrc.iscas.ac.cn>



On 24/12/2025 11:39 am, Haoxiang Li wrote:
> On Wed, 24 Dec 2025 10:39:18 +0000, James Clark wrote:
>> Does this actually do anything? jvmti_close() is already called in
>> Agent_OnUnload().
> 
> I think Agent_OnUnload() is not called if Agent_Onload() fails, so it
> is necessary to release the resource.
> 

The docs say otherwise and suggest that VMDeath() is actually the one 
that wouldn't be called if startup was unsuccessful. But 
Agent_OnUnload() is always called:

  this function will be called if some platform specific mechanism causes
  the unload (an unload mechanism is not specified in this document) or
  the library is (in effect) unloaded by the termination of the VM
  whether through normal termination or VM failure, including start-up
  failure. ...  Note the distinction between this function and the VM
  Death event: for the VM Death event to be sent, the VM must have run at
  least to the point of initialization

>> The commit message is lacking any details about how this was found or
>> what the effect is.
> 
> Sorry for that. I found it by a static analyzer prototype and comfirmed
> by manual review. I think it leads to a resource leak.
> 
> If this is ok, I modify the changelog and resubmit it.
> 

I don't think it's enough, you have to actually run the code that you 
submit. For all we know it results in some double free and makes it worse.

> Thanks,
> Haoxiang Li
> 



      reply	other threads:[~2025-12-24 11:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-24  3:56 [PATCH] perf jit: close agent in Agent_OnLoad() Haoxiang Li
2025-12-24 10:39 ` James Clark
2025-12-24 11:39   ` Haoxiang Li
2025-12-24 11:48     ` James Clark [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ef9af950-84d7-408c-8f1d-e9e75c6dc561@linaro.org \
    --to=james.clark@linaro.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=lihaoxiang@isrc.iscas.ac.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=yuzhuo@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox