Skip to content

fix(#6640): take heap blocks only through the releasing scope #6668

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.

Sign up for GitHub

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

Open
yangjj-iso wants to merge 1 commit into
base: master
Choose a base branch
from
+98 −87
Open

fix(#6640): take heap blocks only through the releasing scope #6668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 36 additions & 30 deletions eo-runtime/src/main/java/org/eolang/Heaps.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -250,11 +221,46 @@ void write(final int identifier, final int offset, final byte[] data) {
}
}

/**
* Allocate a block in memory.
*
* <p>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.</p>
*
* @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.
*
* <p>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.</p>
*
* @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)) {
Expand Down
8 changes: 4 additions & 4 deletions eo-runtime/src/test/java/org/eolang/EOmallocEOofTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}

Expand All @@ -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"
);
}

Expand Down
111 changes: 58 additions & 53 deletions eo-runtime/src/test/java/org/eolang/HeapsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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);
}

/**
Expand Down
Loading