aionnetwork / AVM

Enabling Java code to run in a blockchain environment
https://theoan.com/
MIT License
49 stars 25 forks source link

[CLOSED] Reentrant write-back incorrectly references constants #254

Closed aionbot closed 5 years ago

aionbot commented 5 years ago

Issue created by jeff-aion (on Wednesday Sep 19, 2018 at 16:04 GMT)

The bug found in 5d4189d7cc3296be09f592e64a9d39f8a8a98a30, while working on #246, should be handled under its own issue since this is actually rather complicated and warrants a more in-depth explanation.

First of all, the bug is not observed in our current tests since we don't re-load a reentrant object graph, in any of our tests, an additional time after running a test against the logic. This problem can be observed in ShadowSerializationTest.testReentrantJavaNio() if the last verifyReentrantChange line is duplicated. In this case, the problem is a failure to assign a constant Boolean to a ByteArray field, which is the error expected based on the bug where normal references are converted to constant references.

aionbot commented 5 years ago

Comment by jeff-aion (on Wednesday Sep 19, 2018 at 16:08 GMT)

After some investigation, the problem seems to be related to the use of LoopbackCodec in ReentrantGraphProcessor.internalCommitGraphToStoredFieldsAndRestore().

Specifically, calleeSpaceToProcess.deserializeSelf is incorrectly creating a stub object, pointing into the callee space, if it doesn't find a caller space instance. This is incorrect since we want to copy-back the reference to the callee space object, in this case.

I suspect that the problem was due to incorrectly reusing too much of this mapping logic, resulting in some reverse logic being applied.

aionbot commented 5 years ago

Comment by jeff-aion (on Wednesday Sep 19, 2018 at 16:14 GMT)

This does appear to be the case. LoopbackCodec takes in a AutomaticSerializer and AutomaticDeserializer, in order to ensure that objects being passed through the codec can be appropriately wrapped/unwrapped and stubbed. This means that, in this case, we are calling ReentrantGraphProcessor.wrapAsStub on an object which we actually want to pass through as literally what it was or what its caller counterpart is.

I suspect that, in this case, we actually want a largely empty implementation of these helpers. I will prototype this and see what that exposes.