Skip to content

Conversation

@chrisnas
Copy link
Contributor

@chrisnas chrisnas commented Jun 24, 2022

In the context of #49424, 3 methods are added to ICorProfilerInfo to manage strong, weak and pinned handles.

The first usage will be to wrap the reference provided by AllocationTick_V4 by a weak reference handle and use them to figure out if the object is still alive after a certain amount of time. This should help building automatic memory leak detection system.

Note: the corresponding documentation is made available via dotnet/docs#30131

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-VM-coreclr labels Jun 24, 2022
@teo-tsirpanis
Copy link
Contributor

@davmason

@ghost
Copy link

ghost commented Jun 25, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

In the context of #55888, 3 methods are added to ICorProfilerInfo to manage strong, weak and pinned handles.

The first usage will be to wrap the reference provided by AllocationTick_V4 by a weak reference handle and use them to figure out if the object is still alive after a certain amount of time. This should help building automatic memory leak detection system.

Author: chrisnas
Assignees: -
Labels:

area-Diagnostics-coreclr, community-contribution

Milestone: -

@chrisnas chrisnas closed this Jun 29, 2022
@chrisnas chrisnas force-pushed the profiler-can-create-handles branch from 75571c0 to 2a38e20 Compare June 29, 2022 16:12
@davmason
Copy link
Contributor

@chrisnas if closing it was by accident, you should be able to reopen it. I can't for some reason

@chrisnas
Copy link
Contributor Author

git error...

@chrisnas chrisnas reopened this Jun 30, 2022
Copy link
Contributor

@davmason davmason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small changes, otherwise looks good to me!

break;

default:
handle = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should return E_INVALIDARG in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Good point. I'm also setting *pHandle to NULL

@ghost ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jul 6, 2022
@davmason
Copy link
Contributor

@chrisnas I think this is ready to merge, when are you planning on doing the docs on docs.microsoft.com? I would really prefer to wait for a docs PR before merging, otherwise docs tend to never get done.

@chrisnas
Copy link
Contributor Author

@chrisnas I think this is ready to merge, when are you planning on doing the docs on docs.microsoft.com? I would really prefer to wait for a docs PR before merging, otherwise docs tend to never get done.

@davmason : is there anything else to do beyond dotnet/docs#30131?

@davmason
Copy link
Contributor

is there anything else to do beyond dotnet/docs#30131?

Nope, just missed that PR. Thanks, I'll merge this now

Copy link
Contributor

@davmason davmason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@davmason davmason merged commit 4d4ddf9 into dotnet:main Jul 16, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Diagnostics-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants