Skip to content

Delete more of ExceptionTracker code #114146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2025

Conversation

filipnavara
Copy link
Member

No description provided.

@filipnavara filipnavara requested a review from janvorli April 2, 2025 11:52
@ghost ghost added the area-VM-coreclr label Apr 2, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 2, 2025
Copy link
Contributor

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

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@janvorli
Copy link
Member

janvorli commented Apr 2, 2025

@filipnavara thank you for making these changes. As a next step, if you still want to proceed with the cleanup, I can see the following steps:

  • move the static methods from the ExceptionTracker to the ExInfo
  • remove the ExceptionTracker completely
  • Merge the ExceptionTrackerBase into the ExInfo

@filipnavara
Copy link
Member Author

Thanks! I expected the next steps to be that. I'll likely get to it at some point but my primary motivation was to simplify code base search for my win-x86/funclet branch and I've already achieved that so the remaining cleanup is no longer high priority for me.

@filipnavara
Copy link
Member Author

Seems like there are some failures:

Assert failure(PID 62 [0x0000003e], Thread: 62 [0x003e]): Reached the "unreachable": 
FAILED: <unreachable>
    File: /__w/1/s/src/coreclr/vm/exceptionhandling.h:231
    Image: /root/helix/work/correlation/dotnet

I'll investigate tonight.

@janvorli
Copy link
Member

janvorli commented Apr 2, 2025

This comes from the ExceptionTracker constructor. I guess it comes from the ExceptionTracker m_OOMTracker; in the ThreadExceptionState

@filipnavara
Copy link
Member Author

This comes from the ExceptionTracker constructor. I guess it comes from the ExceptionTracker m_OOMTracker; in the ThreadExceptionState

Thanks, that was it.

@jkotas jkotas merged commit 1587221 into dotnet:main Apr 3, 2025
96 of 98 checks passed
@filipnavara filipnavara deleted the exception-tracker-dead-code-2 branch April 27, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ExceptionHandling-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.

4 participants