From 22473526af5e587d8b6894a384d5e2c4ec3a9615 Mon Sep 17 00:00:00 2001 From: yangjj-iso <3202137046@qq.com> Date: Wed, 12 Aug 2026 21:35:21 +0800 Subject: [PATCH] fix(#6640): take heap blocks only through the releasing scope Resolves the #6507 puzzle in Heaps. The two-argument malloc handed out a block with nobody responsible for releasing it, and free let any caller release a block it did not own, so the scoped malloc was a convention rather than a rule. Both are private now, which makes the scope the only way in and out. The seven HeapsTest cases that took a block raw and freed it by hand moved onto the scoped form, the two EOmallocEOofTest probes now ask size whether the block survived instead of trying to free it a second time, and failsOnClearingEmptyBlock is gone because no caller can reach free. The private pair sits at the end of the class: qulice orders methods by visibility, and leaving malloc where it was put every package-private method after it out of order. Closes #6640 --- .../src/main/java/org/eolang/Heaps.java | 66 ++++++----- .../java/org/eolang/EOmallocEOofTest.java | 8 +- .../src/test/java/org/eolang/HeapsTest.java | 111 +++++++++--------- 3 files changed, 98 insertions(+), 87 deletions(-) diff --git a/eo-runtime/src/main/java/org/eolang/Heaps.java b/eo-runtime/src/main/java/org/eolang/Heaps.java index 35e098760c7..589906cb597 100644 --- a/eo-runtime/src/main/java/org/eolang/Heaps.java +++ b/eo-runtime/src/main/java/org/eolang/Heaps.java @@ -14,10 +14,6 @@ /** * Dynamic memory. * @since 0.19 - * @todo #6507:30min Move the negative-argument, size and resize tests in HeapsTest and - * both free probes in EOmallocEOofTest onto the scoped malloc, then make malloc with two - * arguments and free private, so that a block can only be taken through a scope that - * releases it, and drop failsOnClearingEmptyBlock, which nobody can reach any more. */ final class Heaps { @@ -45,31 +41,6 @@ private Heaps() { this.lock = new ReentrantLock(); } - /** - * Allocate a block in memory. - * @param phi Object - * @param size How many bytes - * @return The identifier of pointer to the block in memory - */ - int malloc(final Phi phi, final int size) { - final int identifier = phi.hashCode(); - this.lock.lock(); - try { - if (this.blocks.containsKey(identifier)) { - throw new ExFailure( - String.format( - "Can't allocate block in memory with identifier '%d' because it's already allocated", - identifier - ) - ); - } - this.blocks.put(identifier, new byte[size]); - } finally { - this.lock.unlock(); - } - return identifier; - } - /** * Allocate a block in memory, let the scope use it, and free it afterwards. * @param phi Object @@ -250,11 +221,46 @@ void write(final int identifier, final int offset, final byte[] data) { } } + /** + * Allocate a block in memory. + * + *

Private on purpose: a block handed out here has no owner responsible + * for releasing it. Take one through {@link #malloc(Phi, int, IntFunction)} + * instead, whose scope frees the block on every exit path.

+ * + * @param phi Object + * @param size How many bytes + * @return The identifier of pointer to the block in memory + */ + private int malloc(final Phi phi, final int size) { + final int identifier = phi.hashCode(); + this.lock.lock(); + try { + if (this.blocks.containsKey(identifier)) { + throw new ExFailure( + String.format( + "Can't allocate block in memory with identifier '%d' because it's already allocated", + identifier + ) + ); + } + this.blocks.put(identifier, new byte[size]); + } finally { + this.lock.unlock(); + } + return identifier; + } + /** * Free it. + * + *

Private on purpose: the scope opened by + * {@link #malloc(Phi, int, IntFunction)} is the only thing that releases a + * block, so no caller can free one it does not own or free one twice.

+ * * @param identifier Identifier of pointer */ - void free(final int identifier) { + private void free(final int identifier) { this.lock.lock(); try { if (!this.blocks.containsKey(identifier)) { diff --git a/eo-runtime/src/test/java/org/eolang/EOmallocEOofTest.java b/eo-runtime/src/test/java/org/eolang/EOmallocEOofTest.java index b0f1fa9478f..43912694098 100644 --- a/eo-runtime/src/test/java/org/eolang/EOmallocEOofTest.java +++ b/eo-runtime/src/test/java/org/eolang/EOmallocEOofTest.java @@ -27,8 +27,8 @@ void freesMemory() { ).take(); Assertions.assertThrows( ExAbstract.class, - () -> Heaps.INSTANCE.free((int) dummy.id), - "Heaps should throw an exception on attempt to free already freed memory, but it didn't" + () -> Heaps.INSTANCE.size((int) dummy.id), + "Heaps should have released the block when the scope ended, but it was still allocated" ); } @@ -47,8 +47,8 @@ void freesMemoryIfErrorIsOccurred() { ); Assertions.assertThrows( ExAbstract.class, - () -> Heaps.INSTANCE.free((int) dummy.id), - "Heaps should throw an exception on attempting to free already freed memory after failure, but it didn't" + () -> Heaps.INSTANCE.size((int) dummy.id), + "Heaps should have released the block after the scope failed, but it was still allocated" ); } diff --git a/eo-runtime/src/test/java/org/eolang/HeapsTest.java b/eo-runtime/src/test/java/org/eolang/HeapsTest.java index b3c065f54a4..741dd9a95f0 100644 --- a/eo-runtime/src/test/java/org/eolang/HeapsTest.java +++ b/eo-runtime/src/test/java/org/eolang/HeapsTest.java @@ -107,19 +107,22 @@ void failsCleanlyOnWriteWithNegativeOffset() { @Test void keepsBlockIntactAfterWriteWithNegativeOffset() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 3); - Heaps.INSTANCE.write(idx, 0, new byte[] {7, 8, 9}); - Assertions.assertThrows( - ExFailure.class, - () -> Heaps.INSTANCE.write(idx, -2, new byte[] {1, 2}), - "Heaps must reject a negative write offset before touching the block, but it didn't" - ); MatcherAssert.assertThat( "Heaps must leave the block untouched after a rejected negative-offset write, but it didn't", - Heaps.INSTANCE.read(idx, 0, 3), + Heaps.INSTANCE.malloc( + new HeapsTest.PhFake(), 3, + idx -> { + Heaps.INSTANCE.write(idx, 0, new byte[] {7, 8, 9}); + Assertions.assertThrows( + ExFailure.class, + () -> Heaps.INSTANCE.write(idx, -2, new byte[] {1, 2}), + "Heaps must reject a negative write offset before touching the block, but it didn't" + ); + return Heaps.INSTANCE.read(idx, 0, 3); + } + ), Matchers.equalTo(new byte[] {7, 8, 9}) ); - Heaps.INSTANCE.free(idx); } @Test @@ -156,18 +159,21 @@ void failsOnReadIfOffsetPlusLengthOverflows() { @Test void failsCleanlyOnNegativeReadArguments() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 10); - Assertions.assertThrows( - ExFailure.class, - () -> Heaps.INSTANCE.read(idx, -5, 3), - "Heaps must reject a negative offset with a clean ExFailure, not a raw JVM exception" + Heaps.INSTANCE.malloc( + new HeapsTest.PhFake(), 10, + idx -> { + Assertions.assertThrows( + ExFailure.class, + () -> Heaps.INSTANCE.read(idx, -5, 3), + "Heaps must reject a negative offset with a clean ExFailure, not a raw JVM exception" + ); + return Assertions.assertThrows( + ExFailure.class, + () -> Heaps.INSTANCE.read(idx, 2, -3), + "Heaps must reject a negative length with a clean ExFailure, not a raw JVM exception" + ); + } ); - Assertions.assertThrows( - ExFailure.class, - () -> Heaps.INSTANCE.read(idx, 2, -3), - "Heaps must reject a negative length with a clean ExFailure, not a raw JVM exception" - ); - Heaps.INSTANCE.free(idx); } @Test @@ -256,15 +262,6 @@ void freesSuccessfully() { ); } - @Test - void failsOnClearingEmptyBlock() { - Assertions.assertThrows( - ExFailure.class, - () -> Heaps.INSTANCE.free(new HeapsTest.PhFake().hashCode()), - "Heaps should throw an exception on attempting to free a non-existent block, but it didn't" - ); - } - @Test void throwsOnGettingSizeOfEmptyBlock() { Assertions.assertThrows( @@ -276,24 +273,23 @@ void throwsOnGettingSizeOfEmptyBlock() { @Test void returnsValidSize() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 5); MatcherAssert.assertThat( "Heaps should return valid size of allocated block, but it didn't", - Heaps.INSTANCE.size(idx), + Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 5, Heaps.INSTANCE::size), Matchers.equalTo(5) ); - Heaps.INSTANCE.free(idx); } @Test void throwsOnChangingSizeToNegative() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 5); - Assertions.assertThrows( - ExFailure.class, - () -> Heaps.INSTANCE.resize(idx, -1), - "Heaps should throw an exception on trying to changing size to negative, but it didn't" + Heaps.INSTANCE.malloc( + new HeapsTest.PhFake(), 5, + idx -> Assertions.assertThrows( + ExFailure.class, + () -> Heaps.INSTANCE.resize(idx, -1), + "Heaps should throw an exception on trying to changing size to negative, but it didn't" + ) ); - Heaps.INSTANCE.free(idx); } @Test @@ -307,41 +303,50 @@ void throwsOnChangeSizeOfEmtpyBlock() { @Test void increasesSizeSuccessfully() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 5); - Heaps.INSTANCE.write(idx, 0, new byte[] {1, 2, 3, 4, 5}); - Heaps.INSTANCE.resize(idx, 7); MatcherAssert.assertThat( "Heaps should successfully increase size of allocated block, but it didn't", - Heaps.INSTANCE.read(idx, 0, 7), + Heaps.INSTANCE.malloc( + new HeapsTest.PhFake(), 5, + idx -> { + Heaps.INSTANCE.write(idx, 0, new byte[] {1, 2, 3, 4, 5}); + Heaps.INSTANCE.resize(idx, 7); + return Heaps.INSTANCE.read(idx, 0, 7); + } + ), Matchers.equalTo(new byte[] {1, 2, 3, 4, 5, 0, 0}) ); - Heaps.INSTANCE.free(idx); } @Test void decreasesSizeSuccessfully() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 5); - Heaps.INSTANCE.write(idx, 0, new byte[]{1, 2, 3, 4, 5}); - Heaps.INSTANCE.resize(idx, 3); MatcherAssert.assertThat( "Heaps should successfully decrease size of allocated block, but it didn't", - Heaps.INSTANCE.read(idx, 0, 3), + Heaps.INSTANCE.malloc( + new HeapsTest.PhFake(), 5, + idx -> { + Heaps.INSTANCE.write(idx, 0, new byte[] {1, 2, 3, 4, 5}); + Heaps.INSTANCE.resize(idx, 3); + return Heaps.INSTANCE.read(idx, 0, 3); + } + ), Matchers.equalTo(new byte[] {1, 2, 3}) ); - Heaps.INSTANCE.free(idx); } @Test void returnsValidSizeAfterDecreasing() { - final int idx = Heaps.INSTANCE.malloc(new HeapsTest.PhFake(), 5); - Heaps.INSTANCE.write(idx, 0, new byte[]{1, 2, 3, 4, 5}); - Heaps.INSTANCE.resize(idx, 3); MatcherAssert.assertThat( "Heaps should return valid size after decreasing, but it didn't", - Heaps.INSTANCE.size(idx), + Heaps.INSTANCE.malloc( + new HeapsTest.PhFake(), 5, + idx -> { + Heaps.INSTANCE.write(idx, 0, new byte[] {1, 2, 3, 4, 5}); + Heaps.INSTANCE.resize(idx, 3); + return Heaps.INSTANCE.size(idx); + } + ), Matchers.equalTo(3) ); - Heaps.INSTANCE.free(idx); } /**