Skip to content

Fix RawMemoryFreeList panicking when it grows up to its limit - #1627

Open
Ran-Mewo wants to merge 2 commits into
mmtk:masterfrom
Ran-Mewo:fix-raise-high-water
Open

Ran-Mewo wants to merge 2 commits into
mmtk:masterfrom
Ran-Mewo:fix-raise-high-water

Conversation

@Ran-Mewo

@Ran-Mewo Ran-Mewo commented Nov 3, 2026 •

Copy link
Copy Markdown

Growing a RawMemoryFreeList up to its limit panics. raise_high_water subtracted the wrong way round when clamping to the limit, and current_capacity ignored the last block when the limit cut it short. Easy to hit with a small fixed heap, e.g. 24 MB.

@Ran-Mewo Ran-Mewo changed the title Fix grow extent underflow in RawMemoryFreeList::raise_high_water Fix RawMemoryFreeList growth near its limit Nov 3, 2026
@Ran-Mewo Ran-Mewo changed the title Fix RawMemoryFreeList growth near its limit Fix RawMemoryFreeList panicking when it grows up to its limit Nov 3, 2026
"Attempt to grow FreeList beyond limit"
);
if self.high_water + grow_extent > self.limit {
grow_extent = self.high_water - self.limit;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like the code in the Rust MMTk core is ported from this piece of code in JikesRVM:

  /**
   * Raise the high water mark by requesting more pages from the OS
   *
   * @param blocks block count to raise by
   */
  private void raiseHighWater(int blocks) {
    Extent growExtent = Conversions.pagesToBytes(pagesPerBlock * blocks);
    if (highWater.EQ(limit)) {
      Log.write("limit=", limit);
      Log.write(", highWater=", highWater);
      Log.writeln(", growExtent=", growExtent);
      VM.assertions.fail("Attempt to grow FreeList beyond limit");
    }
    if (highWater.plus(growExtent).GT(limit)) {
      growExtent = highWater.diff(limit).toWord().toExtent();
    }
    mmap(highWater, growExtent);
    highWater = highWater.plus(growExtent);
  }

highWater.diff(limit) means highWater - limit. The highWater should not be higher than than limit, and the result of the diff shall be negative. It is a bug in JikesRVM, and I am surprised it has existed since 2016 and never gets found.

@k-sareen

k-sareen commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Looks like this PR and #1625 are both trying to fix the same issue? Or at least very similar issues

@qinsoon

qinsoon commented Nov 4, 2026

Copy link
Copy Markdown
Member

Looks like this PR and #1625 are both trying to fix the same issue? Or at least very similar issues

This PR seems to be a subset of #1625 with one additional test.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants