Hello
In GraalVM CE 19.2.0, I have jsonObject build in Java using ProxyObject and passed to GraalVM Context. Then I want to update in Java script guest code and got an exception.
...
//Java on host
try (Context jsContext =
Context.newBuilder("js").allowHostAccess(HostAccess.EXPLICIT).build()) {
jsContext.getBindings("js").putMember("jsonObject", initDataProxy);
jsContext.eval("js", jsCode);
}
}
...
//Java script on guest
for (var key in jsonObject) {
jsonObject[key] = {'a':'b'};
}
...
Here exception message:
Exception in thread "main" TypeError: writeArrayElement on foreign object failed due to: Message not supported.
at <js> :program(Unnamed:1:1557-1583)
at org.graalvm.polyglot.Context.eval(Context.java:370)
at com.researchforgood.survey.surveyengine.service.JSProxyBuilderTest.performanceTest(JSProxyBuilderTest.java:174)
at com.researchforgood.survey.surveyengine.service.JSProxyBuilderTest.main(JSProxyBuilderTest.java:265)
As you see I cannot assign object data in JS code. Am I doing something wrong and this is documented somehow? How can I assign data?
Hi @pavlomorozov
thanks for your question. Can you please iterate a bit more what you send into the proxy object? I suspect the problem to be there, i.e. that the proxy cannot update the underlying data structure.
When using a simple Map<String, Object> there it is working fine:
import java.util.HashMap;
import java.util.Map;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.HostAccess;
import org.graalvm.polyglot.proxy.ProxyObject;
public class Main {
public static void main(String[] args) {
String jsCode = "for (var key in jsonObject) { \n " +
" print('setting: '+key); \n" +
" jsonObject[key] = {'a':'b'}; \n" +
"}; ";
Map<String, Object> jsonObject = new HashMap<>();
jsonObject.put("foo", "aaa");
ProxyObject initDataProxy = ProxyObject.fromMap(jsonObject);
try (Context jsContext =
Context.newBuilder("js").allowHostAccess(HostAccess.EXPLICIT).build()) {
jsContext.getBindings("js").putMember("jsonObject", initDataProxy);
jsContext.eval("js", jsCode);
System.out.println(jsonObject.get("foo"));
System.out.println(jsonObject.get("wrong"));
}
}
}
output on my machine as expected:
setting: foo
{a: "b"}
null
Best,
Christian
The logic for proxyObject[key] = value is quite simple: if the key is a string, we send _writeMember_ (i.e. ProxyObject#putMember), and if it's a number, we send _writeArrayElement_ to the object. Note that ProxyObject member keys must be strings.
I was able to track down the problem. It happen if object property is a string with number in it. You can see the same if change your code
jsonObject.put("foo", "aaa");
to
jsonObject.put("1", "aaa");
@woess Just saw you message. As you can see I use String data type in Java. So I think it would be natural to have JavaScript object available like :
{1, "aaa"}
And it would be great to have same logic when JS object build in JS code passed back to Java in Value object.
Hi @pavlomorozov
I can reproduce your problem. We are investigating.
Best,
Christian
We were just bit by this problem as well. Graal 19.1.0 seems to be the first release where this issue occurs. Graal 19.0.2 works fine, and is currently the most recent version we can put into production because of this issue.
Here is our simple test using Groovy and testng:
@Test
void "Graal issue 206"() {
def context = Context.newBuilder('js').build()
context.getBindings('js').putMember('proxy', ProxyObject.fromMap([:]))
// Graal 19.1.0+ throws "TypeError: writeArrayElement on foreign object..." :
context.eval('js', 'proxy["0"] = "zero";')
}
Hi @wirthi Any update on this? We'd love to use some of the newer Graal features (looking at you, Resource Limits) but this bug looks to be a deal-breaker for moving beyond 19.0.2.
@wirthi, will this be fixed in 19.3.0? Thanks!
@wirthi, this is also preventing our company from upgrading a service that uses Graal. We were at 19.0.0 and just attempted the upgrade and our integration test suite failed because of this.
To clarify this will produce the error:
let index = createProxyObject(); // This is a function in Java that creates an Object that implements ProxyObject
index["12345"] = "ABC"; // This will generate the error
index["x12345"] = "ABC"; // This will work (note, just put an X at the beginning so this is no longer numeric)
index["123x45"] = "ABC"; // This will work as well, just need 1 letter to make it work
index[" 12345"] = "ABC"; // This also work by just putting a space at the beginning
I debugged the issue in GraalVM 19.3.0 and found the following:
WriteElementNode.executeWithTarget which gets called correctly (parameters look ok based on it being called as a result of myVar[x] syntax in JS)index returned by indexNode.execute(frame) will correctly return a String toArrayIndexNode().execute(index) converts the String to a Long (by ToArrayIndexNode.convertFromString), which ultimately causes the downstream code to failWriteElementNode$TruffleObjectWriteElementTypeCacheNode.executeWithTargetAndIndexUnguarded is called the index parameter will be a Long (instead of a String)keyInterop.fitsInLong(convertedKey) condition causing interop.writeArrayElement to be called, triggering the error as logged in this ticketIt appears the fix could go in ToArrayIndexNode.convertFromString, but this seems dangerous as it does not seem to have enough information to make the conversion (i.e. it does not know the type of the object being operated on). It seems a better solution would be to put a fix in WriteElementNode$TruffleObjectWriteElementTypeCacheNode.executeWithTargetAndIndexUnguarded where you know the type of the object being operated on (i.e. the type of the truffleObject which will be an instanceof ProxyObject, ProxyArray, etc.). So maybe a check like this would work:
TruffleObject truffleObject = targetClass.cast(target);
...
if (keyInterop.isString(convertedKey) || interop.hasMembers(truffleObject)) {
...
interop.writeMember(truffleObject, keyInterop.asString(convertedKey), exportedValue);
...
} else if (keyInterop.fitsInLong(convertedKey)) {
...
interop.writeArrayElement(truffleObject, keyInterop.asLong(convertedKey), exportedValue);
My other concern is that there may be other locations (other then WriteElementNode$TruffleObjectWriteElementTypeCacheNode.executeWithTargetAndIndexUnguarded), which may have similar issues and all would need to have a similar fix applied.
NOTE: The error is thrown from interop.writeArrayElement when it detects that truffleObject is not a ProxyArray. So as a workaround I updated my Java class that implements ProxyObject to also implement ProxyArray and implemented ProxyArray.set as follows:
@Override
public void set(long index, Value value) {
putMember(Long.toString(index), value); // Convert the long to String and call ProxyObject.putMember
}
This works as long as you don't call any function that has to make a decision on if to iterate members or the array. For example, after making the above change, the integration tests that were breaking were fixed. But other integration tests broke. Specifically there were test that were calling JSON.stringify(o), which failed as GraalVM seems to give preference to iterating ProxyArray, so my "Object" was turned into a JSON List instead of a JSON Object.
The root of the problem is that JavaScript (unlike the interop API) does not distinguish numeric and string properties. object[42] and object['42'] are the same in JavaScript but there are two methods in the interop API (writeMember() and writeArrayElement()) that can be mapped to this JavaScript operation.
Our current rule/heuristics (described by @woess above): "if the key is a string, we send writeMember() and if it's a number, we send writeArrayElement()" does not work well when the key is (or can be converted to) a number but the corresponding object does not have array elements (like a typical ProxyObject). I think that we should modify this rule such that writeArrayElement() is called on an object that actually hasArrayElements() only. Otherwise the key should be converted to string and writeMember() should be used instead. This change would ensure that proxyArray[42] would trigger writeArrayElement() still but both proxyObject[42] and proxyObject['42'] would work as expected.
@iamstolis - thanks for taking a look.
Regardless of how JavaScript distinguishes numeric and string properties, GraalJS is not behaving as such. For example, I just tested GraalJS with the following JS code:
index["12345"] = "ABC"; // NOTE: JS string type that happens to be number
index[12345] = "ABC"; // NOTE: JS number
And ran this through the debugger. And WriteElementNode.executeWithTarget after the call to indexNode.execute(frame) to create the index "Object" local variable will return a String (i.e. "12345") on the first case (JS string type that happens to be a number), but returns an Integer (i.e. 12345) on the second case (JS number). So a different downstream flow will be invoked as a result (as shown here):
public Object executeWithTarget(VirtualFrame frame, Object target, Object receiver) {
...
if (index instanceof Integer) {
// NOTE: second case above hits this block
indexState = INDEX_INT;
return executeWithTargetAndIndex(frame, target, (int) index, receiver);
} else {
// NOTE: first case above hits this block
indexState = INDEX_OBJECT;
return executeWithTargetAndIndex(frame, target, toArrayIndexNode().execute(index), receiver);
}
The point is, the interop code does seem to be able to distinguish very early in the processing of the index the JS type (I apologize, if my terminology is wrong here, as this is the first time I've looked at the GraalJS code in any depth).
Regardless, still not sure what the best way to fix this would be. It appears your suggested solution is the same as the solution I suggested except you are checking if it has array elements vs. checking if it has members (which seems to be the order used by other operations, such as JSON.stringify). I really don't think it matters as I think both achieve the same result.
I also should have been clearer in my earlier comment that when I retested with "a change", I did not update the GraalJS code. I simply tested the "workaround" of having my Java object implement both ProxyObject and ProxyArray.
As it stands, I must revert our service back to Graal 19.0.x until this issue is resolved. This is definitely a blocker for us.
Thinking about this some more, since GraalJS knows about the type of the key fairly early in the processing of the statement, it could be fairly smart on how it handles it.
The following algorithm seems ideal:
if (key is string) {
// If key is a string give preference to ProxyObject
if (truffleObject has members) {
writeMember(key, value)
} else if (truffleObject has array elements) {
writeArrayElement(toNumber(key), value)
}
} else if (key is numeric) {
// if key is numeric give preference to ProxyArray
if (truffleObject has array elements) {
writeArrayElement(key, value)
} else if (truffleObject has members) {
writeMember(toString(key), value)
}
}
This is likely the breaking commit:
https://github.com/graalvm/graaljs/commit/e53ed189c33ecacd94bc02f276e034e693510c02#diff-0eb397dd09946440a5ced913b92ce667
Line 243 was;
return executeWithTargetAndIndex(frame, target, index);
Change to:
return executeWithTargetAndIndex(frame, target, toArrayIndexNode().execute(index));
In the old working code index remained a string so the downstream code worked. In the updated code the toArrayIndexNode().execute(index) converts index to a Long causing downstream code to fail.
I apologize, I don't mean to keep spamming this thread, but want to get as much information out there to get this fixed.
I downgraded to GraalVM 19.1.1 expecting the issue to not exist based on what seemed to be the breaking commit mentioned above. This version has the bug as well but for a different reason.
In GraalJS 19.0.x and 19.3.0, the Object index = indexNode.execute(frame) call in WriteElementNode.executeWithTarget returns the correct type and 19.0.x does not have a call toArrayIndexNode().execute(index) so it keeps the "String" type and therefore works (where in GraalJS 19.3.0 the "String" is converted to "Long" which causes it to fail).
In GraalJS 19.1.1, the call to Object index = indexNode.execute(frame) returns a Long in this case so even though it does not call toArrayIndexNode().execute(index), the downstream code still fails.
So the last working version is 19.0.2.
@iamstolis We are looking forward to the fix being merged. Thank you!
This issue should be fixed by https://github.com/graalvm/graaljs/commit/be1e2925c6fa5619daffb160e9eb55fb14a147aa
Most helpful comment
Hi @pavlomorozov
I can reproduce your problem. We are investigating.
Best,
Christian