Skip to content

Commit

Permalink
Fix #348: Moved lazyLoad() calls out of persistence constructors
Browse files Browse the repository at this point in the history
-these could cause issues where an OutOfEnergy would be thrown from a constructor being called by the persistence layer, through reflection (which is not a case we should handle)
-this "eager lazy loading" implementation also would cause over-billing since just referencing one of these types would cause it to be loaded
-the lazyLoad() is now only called before an instance variable is accessed, as it should be
-added some tests to make sure that we can fail at deserializing each object in our serialization test
  • Loading branch information
jeff-aion committed Feb 8, 2019
1 parent 293178d commit 03275e4
Show file tree
Hide file tree
Showing 12 changed files with 328 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,78 @@ public void testReentrantApi() {
verifyReentrantChange(contractAddr, "Api");
}

@Test
public void testEnergyLoadingJavaLang() {
byte[] txData = avmRule.getDappBytes(ShadowCoverageTarget.class, new byte[0]);

// deploy
TransactionResult result1 = avmRule.deploy(deployer, BigInteger.ZERO, txData, DEPLOY_ENERGY_LIMIT, ENERGY_PRICE).getTransactionResult();
Assert.assertEquals(AvmTransactionResult.Code.SUCCESS, result1.getResultCode());
Address contractAddr = TestingHelper.buildAddress(result1.getReturnData());

// Populate initial data.
int firstHash = populate(contractAddr, "JavaLang");
// For now, just do the basic verification based on knowing the number.
Assert.assertEquals(HASH_JAVA_LANG, firstHash);

long energyLimit = 0;
int hash = 0;
while (0 == hash) {
hash = getHashSuccessWithLimit(contractAddr, "JavaLang", energyLimit);
// Allow at most one more read to succeed on the next attempt.
energyLimit += InstrumentationBasedStorageFees.FIXED_READ_COST;
}
Assert.assertEquals(firstHash, hash);
}

@Test
public void testEnergyLoadingJavaMath() {
byte[] txData = avmRule.getDappBytes(ShadowCoverageTarget.class, new byte[0]);

// deploy
TransactionResult result1 = avmRule.deploy(deployer, BigInteger.ZERO, txData, DEPLOY_ENERGY_LIMIT, ENERGY_PRICE).getTransactionResult();
Assert.assertEquals(AvmTransactionResult.Code.SUCCESS, result1.getResultCode());
Address contractAddr = TestingHelper.buildAddress(result1.getReturnData());

// Populate initial data.
int firstHash = populate(contractAddr, "JavaMath");
// For now, just do the basic verification based on knowing the number.
Assert.assertEquals(HASH_JAVA_MATH, firstHash);

long energyLimit = 0;
int hash = 0;
while (0 == hash) {
hash = getHashSuccessWithLimit(contractAddr, "JavaMath", energyLimit);
// Allow at most one more read to succeed on the next attempt.
energyLimit += InstrumentationBasedStorageFees.FIXED_READ_COST;
}
Assert.assertEquals(firstHash, hash);
}

@Test
public void testEnergyLoadingApi() {
byte[] txData = avmRule.getDappBytes(ShadowCoverageTarget.class, new byte[0]);

// deploy
TransactionResult result1 = avmRule.deploy(deployer, BigInteger.ZERO, txData, DEPLOY_ENERGY_LIMIT, ENERGY_PRICE).getTransactionResult();
Assert.assertEquals(AvmTransactionResult.Code.SUCCESS, result1.getResultCode());
Address contractAddr = TestingHelper.buildAddress(result1.getReturnData());

// Populate initial data.
int firstHash = populate(contractAddr, "Api");
// For now, just do the basic verification based on knowing the number.
Assert.assertEquals(HASH_API, firstHash);

long energyLimit = 0;
int hash = 0;
while (0 == hash) {
hash = getHashSuccessWithLimit(contractAddr, "Api", energyLimit);
// Allow at most one more read to succeed on the next attempt.
energyLimit += InstrumentationBasedStorageFees.FIXED_READ_COST;
}
Assert.assertEquals(firstHash, hash);
}


private int populate(Address contractAddr, String segmentName) {
long energyLimit = 1_000_000L;
Expand All @@ -166,6 +238,14 @@ private int getHash(Address contractAddr, String segmentName) {
return ((Integer)TestingHelper.decodeResult(result)).intValue();
}

private int getHashSuccessWithLimit(Address contractAddr, String segmentName, long energyLimit) {
byte[] argData = ABIEncoder.encodeMethodArguments("getHash_" + segmentName);
TransactionResult result = avmRule.call(deployer, AvmAddress.wrap(contractAddr.unwrap()), BigInteger.ZERO, argData, energyLimit, ENERGY_PRICE).getTransactionResult();
return (AvmTransactionResult.Code.SUCCESS == result.getResultCode())
? ((Integer)TestingHelper.decodeResult(result)).intValue()
: 0;
}

private void verifyReentrantChange(Address contractAddr, String segmentName) {
long energyLimit = 2_000_000L;
byte[] argData = ABIEncoder.encodeMethodArguments("verifyReentrantChange_" + segmentName);
Expand Down
10 changes: 8 additions & 2 deletions org.aion.avm.rt/src/org/aion/avm/api/Address.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ public Address(ByteArray raw) {
*/
public ByteArray avm_unwrap() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Address_avm_unwrap);
lazyLoad();
return this.underlying;
}

@Override
public int avm_hashCode() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Address_avm_hashCode);
lazyLoad();
// Just a really basic implementation.
int code = 0;
for (byte elt : this.underlying.getUnderlying()) {
Expand All @@ -62,11 +64,11 @@ public int avm_hashCode() {
@Override
public boolean avm_equals(IObject obj) {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Address_avm_equals);
lazyLoad();

boolean isEqual = this == obj;
if (!isEqual && (obj instanceof Address)) {
Address other = (Address)obj;
lazyLoad();
other.lazyLoad();
if (this.underlying.length() == other.underlying.length()) {
isEqual = true;
Expand All @@ -83,6 +85,7 @@ public boolean avm_equals(IObject obj) {
@Override
public String avm_toString() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Address_avm_toString);
lazyLoad();
return toHexString(this.underlying.getUnderlying());
}

Expand Down Expand Up @@ -119,6 +122,7 @@ public Address(byte[] raw) {
* @return The raw bytes underneath the address.
*/
public byte[] unwrap() {
lazyLoad();
return this.underlying.getUnderlying();
}

Expand All @@ -127,6 +131,8 @@ public boolean equals(java.lang.Object obj) {
boolean isEqual = this == obj;
if (!isEqual && (obj instanceof Address)) {
Address other = (Address) obj;
lazyLoad();
other.lazyLoad();
if (this.underlying.length() == other.underlying.length()) {
isEqual = true;
for (int i = 0; isEqual && (i < other.underlying.length()); ++i) {
Expand All @@ -140,6 +146,7 @@ public boolean equals(java.lang.Object obj) {
@Override
public int hashCode() {
int code = 0;
lazyLoad();
for (byte elt : this.underlying.getUnderlying()) {
code += (int)elt;
}
Expand All @@ -149,6 +156,5 @@ public int hashCode() {
// Support for deserialization
public Address(IDeserializer deserializer, IPersistenceToken persistenceToken) {
super(deserializer, persistenceToken);
lazyLoad();
}
}
14 changes: 13 additions & 1 deletion org.aion.avm.rt/src/org/aion/avm/shadow/java/lang/Byte.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,42 +66,50 @@ private Byte(String s) {

public byte avm_byteValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_byteValue);
lazyLoad();
return v;
}

public short avm_shortValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_shortValue);
lazyLoad();
return (short) v;
}

public int avm_intValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_intValue);
lazyLoad();
return (int) v;
}

public long avm_longValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_longValue);
lazyLoad();
return (long) v;
}

public float avm_floatValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_floatValue);
lazyLoad();
return (float) v;
}

public double avm_doubleValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_doubleValue);
lazyLoad();
return (double) v;
}

public String avm_toString() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_toString_1);
lazyLoad();
return new String(java.lang.Byte.toString(this.v));
}

@Override
public int avm_hashCode() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_hashCode);
lazyLoad();
return internalHashCode(this.v);
}

Expand All @@ -115,13 +123,17 @@ public boolean avm_equals(IObject obj) {
boolean isEqual = false;
if (obj instanceof Byte) {
Byte other = (Byte)obj;
lazyLoad();
other.lazyLoad();
isEqual = v == other.v;
}
return isEqual;
}

public int avm_compareTo(Byte anotherByte) {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Byte_avm_compareTo);
lazyLoad();
anotherByte.lazyLoad();
return internalCompare(this.v, anotherByte.v);
}

Expand Down Expand Up @@ -175,12 +187,12 @@ private static int internalToUnsignedInt(byte x) {

public Byte(IDeserializer deserializer, IPersistenceToken persistenceToken) {
super(deserializer, persistenceToken);
lazyLoad();
}

private byte v;

public byte getUnderlying() {
lazyLoad();
return this.v;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,8 @@ public static boolean avm_isMirrored(int codePoint){

public int avm_compareTo(Character anotherCharacter) {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Character_avm_compareTo);
lazyLoad();
anotherCharacter.lazyLoad();
return avm_compare(this.v, anotherCharacter.v);
}

Expand Down Expand Up @@ -589,12 +591,12 @@ public static int avm_codePointOf(String name) {

public Character(IDeserializer deserializer, IPersistenceToken persistenceToken) {
super(deserializer, persistenceToken);
lazyLoad();
}

private char v;

public char getUnderlying() {
lazyLoad();
return this.v;
}

Expand Down
14 changes: 13 additions & 1 deletion org.aion.avm.rt/src/org/aion/avm/shadow/java/lang/Double.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,52 +93,62 @@ public static boolean avm_isFinite(double d) {

public boolean avm_isNaN() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_isNaN_1);
lazyLoad();
return java.lang.Double.isNaN(this.v);
}

public boolean avm_isInfinite() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_isInfinite_1);
lazyLoad();
return internalIsInfinite(v);
}

public String avm_toString()
{
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_toString_1);
lazyLoad();
return internalToString(v);
}

public byte avm_byteValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_byteValue);
lazyLoad();
return (byte) v;
}

public short avm_shortValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_shortValue);
lazyLoad();
return (short) v;
}

public int avm_intValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_intValue);
lazyLoad();
return (int) v;
}

public long avm_longValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_longValue);
lazyLoad();
return (long) v;
}

public float avm_floatValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_floatValue);
lazyLoad();
return (float) v;
}

public double avm_doubleValue() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_doubleValue);
lazyLoad();
return v;
}

public int avm_hashCode() {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_hashCode);
lazyLoad();
return java.lang.Double.hashCode(this.v);
}

Expand All @@ -164,6 +174,8 @@ public static double avm_longBitsToDouble(long bits){

public int avm_compareTo(Double anotherDouble) {
IInstrumentation.attachedThreadInstrumentation.get().chargeEnergy(RuntimeMethodFeeSchedule.Double_avm_compareTo);
lazyLoad();
anotherDouble.lazyLoad();
return java.lang.Double.compare(this.v, anotherDouble.v);
}

Expand Down Expand Up @@ -207,12 +219,12 @@ private static boolean internalIsInfinite(double v) {

public Double(IDeserializer deserializer, IPersistenceToken persistenceToken) {
super(deserializer, persistenceToken);
lazyLoad();
}

private double v;

public double getUnderlying() {
lazyLoad();
return this.v;
}

Expand Down
Loading

0 comments on commit 03275e4

Please sign in to comment.