On Fri, 1 Dec 2023 05:15:15 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Restrict cleanup to obsolete methods only > > I won't be able to review this this week, too snowed in atm. I can take a > look next week. We can always just revert the change if needed. > > Thinking about Skara, I think as long as we have this confusing mixture of > rules (hotspot wants 2 reviewers that are Reviewer/Committer, but some jdk > libs only want one, but then you need two for desktop I think otherwise Phil > gets angry) - we should hard-code the 2-reviewer rule into skara as default > since it affects the lion's share of all changes. @tstuefe I got confused by the Skara tooling. I had a vague memory of some discussions going on about relaxing the requirement of 2 reviewers for some parts to the code base and I thought I was in a good shape seeing the Skara checkbox.  As for not having a review for the final version - I am not that restless. I specifically dismissed the previous review to avoid incidentally integrating based on a review of a version that was not actual. Then I asked @coleenp to re-do the review on the final bits (https://github.com/openjdk/jdk/pull/16662#issuecomment-1827432032) @dholmes-ora >Okay so why does this happen and is it a reasonable thing to be happening? On >the surface it sounds wrong to deallocate anything associated with a live >classloader. This is happening for previous versions of retransformed methods. As long as those methods are still on stack they are kept alive. But once they are not executing they are free to be destroyed. And this is where the problem was happening - the previous versions of methods were being destroyed but the associated jmethodIDs were not updated not to point to what became an invalid memory block. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16662#issuecomment-1838221097
