Skip to content

Replace ArrayList .equals() method with == to compare the objects #447

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
ahmadov wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Replace ArrayList .equals() method with == to compare the objects #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84e5f0e
Replace ArrayList `.equals()` method with `==` to compare the objects
Jan 18, 2019
2b5c997
Add unit tests to MemoryManagerTest
Jan 21, 2019
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
4 changes: 2 additions & 2 deletions src/main/java/com/eclipsesource/v8/utils/MemoryManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class MemoryManager {
private MemoryManagerReferenceHandler memoryManagerReferenceHandler;
private V8 v8;
private ArrayList<V8Value> references = new ArrayList<V8Value>();
private boolean releasing = false;
private boolean releasing = false;
private boolean released = false;

/**
Expand Down Expand Up @@ -73,7 +73,7 @@ public int getObjectReferenceCount() {
public void persist(final V8Value object) {
v8.getLocker().checkThread();
checkReleased();
references.remove(object);
memoryManagerReferenceHandler.v8HandleDisposed(object);

@irbull irbull Jan 19, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you include a test case in the MemoryManagerTest that gets the same V8ObjectTwice (so there are two copies of it) and then one is persisted? After we release the MemoryManager one should still remain. I can help you with this if you need help.

}

/**
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/com/eclipsesource/v8/utils/MemoryManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,4 +315,32 @@ public void v8HandleCreated(final V8Value object) {
assertTrue(memoryManager.isReleased());
}

@Test
public void testPersistedObjectAffectReferenceCount() {
MemoryManager memoryManager = new MemoryManager(v8);
V8Object object = new V8Object(v8);

assertEquals(1, memoryManager.getObjectReferenceCount());
memoryManager.persist(object);
assertEquals(0, memoryManager.getObjectReferenceCount());

memoryManager.release();
object.close();
}

@Test
public void testPersistDuplicatedObject() {
MemoryManager memoryManager = new MemoryManager(v8);
V8Object object1 = new V8Object(v8);
V8Object object2 = object1.twin();

memoryManager.persist(object2);
memoryManager.release();

assertTrue(object1.isReleased());
assertFalse(object2.isReleased());
object1.close();
object2.close();
}

}