Skip to content
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.

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

XMLHttpRequest broken in Dartium #3466

Closed
DartBot opened this issue Jun 8, 2012 · 10 comments
Closed

XMLHttpRequest broken in Dartium #3466

DartBot opened this issue Jun 8, 2012 · 10 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Milestone

Comments

@DartBot
Copy link

DartBot commented Jun 8, 2012

This issue was originally filed by sammcca...@google.com


In latest build (dartium-lucid64-inc-8445.8445)

Internal error: '/mnt/data/b/build/slave/dartium-lucid64-inc/build/src/out/Release/obj/gen/webkit/bindings/dart/dart/XMLHttpRequestImplementation.dart': Error: line 6 pos 894: native function 'XMLHttpRequest_setRequestHeader_Callback' cannot be found
_addEventListener_1@33cc944a ( type , listener ) ; return ; } _addEventListener_2@33cc944a ( type , listener , useCapture ) ; } void _addEventListener_1@33cc944a ( type , listener ) native XMLHttpRequest_addEventListener_1_Callback ; void _addEventListener_2@33cc944a ( type , /samples/schedule/schedule.dart:1

Build dartium-lucid64-inc-8053.8053 worked fine.

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by antonm@google.com


Sam,

may you provide more details?

Do you create XMLHttpRequest in DOM-enabled isolate?

Full snippet is really appreciated.


Added Area-Dartium, NeedsInfo labels.

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by sammcca...@google.com


Apologies for the lack of detail, I assumed the resolver failure meant it was completely broken.

I'm not creating any isolates.

The full program is complex and includes obfuscated javascript not under my control, I can send it to you internally if that helps.

It's not easy for me to identify what part is failing, as Dartium doesn't provide a stack trace, and simply points at line 1 of my source file.

I can try to cut it down to a simple example by trial and error.

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by antonm@google.com


Sam,

do you run Dart natively or compile it to JS first?

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by sammcca...@google.com


This is natively.
When compiled to JS I use regular chrome, and it works (well, there are different bugs :-)

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by antonm@google.com


I see, thanks. If you can give me a pointer to the app, even messy one, I should be able to debug it.

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by antonm@google.com


That looks like DartVM bug, but please, reassign, if I am wrong.

Assigning to Siva for further dispatches.

In my case native resolver is invoked for instance method declared as:

void setRequestHeader(String header, String value) native "XMLHttpRequest_setRequestHeader_Callback";

with argument count 2 and here Dartium fails to dispatch as it expects 3 args method (this and two remaining args.)

How to debug (on Linux):

  1. Build Debug version of Dartium (x64 should work as well);
  2. Start Dartium with a request to attach gdb to each renderer process:

./out/Debug/chrome --disable-hang-monitor --disable-seccomp-sandbox --renderer-cmd-prefix="xterm -e gdb --args" --user-data-dir http://tunafish.muc.corp.google.com/samples/schedule/

  1. when gdb shows up, put a break point at the very end of generated XMLHttpReqest dispatcher (file is out/Debug/obj/gen/webkit/bindings/dart/cpp/DartXMLHttpRequest.cpp, and gdb should be able to find it as just DartXMLHttpRequest.cpp), the dispatcher is at the very end and bailout for failed search is currently at line 602.

  2. run Dartium and watch for strings which come to this resolver, the following works for me in gdb to print WebCore strings:

 p (*name.ascii().m_buffer.m_ptr).m_vector.m_buffer

  1. after a couple of hits, you should see that "XMLHttpRequest_setRequestHeader_Callback" comes with argumentCount == 2.

Set owner to @a-siva.
Removed Area-Dartium label.
Added Area-VM, Accepted labels.

@DartBot
Copy link
Author

DartBot commented Jun 8, 2012

This comment was originally written by antonm@google.com

@a-siva
Copy link
Contributor

a-siva commented Jun 9, 2012

We seem to be creating an implicit closure function of setRequestHeader somewhere. Since we don't count 'this' in the number of parameters for an implicit closure function (it is a captured variable in the context) we end up with argument count being 2 when we come down to resolve the native function.

A simple test below demonstrates the issue:

void MyNativeClosure(Dart_NativeArguments args) {
  Dart_EnterScope();
  int i = Dart_GetNativeArgumentCount(args);
  Dart_SetReturnValue(args, Dart_NewInteger(i));
  Dart_ExitScope();
}

static Dart_NativeFunction MyNativeClosureResolver(Dart_Handle name,
                                                   int arg_count) {
  if (arg_count == 2) {
    return &MyNativeClosure;
  } else {
    UNREACHABLE();
    return NULL;
  }
}

TEST_CASE(NativeFunctionClosure) {
  const char* kScriptChars =
      "class Test {"
      " int foo(int i) native "SomeNativeClosure";"
      " int bar(int i) { var func = foo; func(i); }"
      "}"
      "int testMain() {"
      " Test obj = new Test();"
      " return obj.foo(1) + obj.bar(1);"
      "}";

  Dart_Handle result;

  // Load a test script.
  Dart_Handle url = Dart_NewString(TestCase::url());
  Dart_Handle source = Dart_NewString(kScriptChars);
  result = Dart_SetLibraryTagHandler(library_handler);
  EXPECT_VALID(result);
  Dart_Handle lib = Dart_LoadScript(url, source);
  EXPECT_VALID(lib);
  EXPECT(Dart_IsLibrary(lib));
  result = Dart_SetNativeResolver(lib, &MyNativeClosureResolver);
  EXPECT_VALID(result);

  result = Dart_Invoke(lib, Dart_NewString("testMain"), 0, NULL);
  EXPECT_VALID(result);
  EXPECT(Dart_IsInteger(result));
  int64_t value = 0;
  EXPECT_VALID(Dart_IntegerToInt64(result, &value));
  EXPECT_EQ(2, value);
}

The spec is not clear on whether it is legal to create an implicit closure of a native function, we should probably not allow that.

@a-siva
Copy link
Contributor

a-siva commented Jun 13, 2012

Added this to the M1 milestone.

@a-siva
Copy link
Contributor

a-siva commented Jun 20, 2012

@DartBot DartBot added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. labels Jun 20, 2012
@DartBot DartBot added this to the M1 milestone Jun 20, 2012
copybara-service bot pushed a commit that referenced this issue Jun 17, 2022
Changes:
```
> git log --format="%C(auto) %h %s" c4e9ddc..9bf4289
 https://dart.googlesource.com/pub.git/+/9bf4289d Top-level --colors flag (#3467)
 https://dart.googlesource.com/pub.git/+/54eb1851 Hide --trace in the embedding (#3466)
 https://dart.googlesource.com/pub.git/+/b9751fdb Fix stack-trace expectation (#3460)

```

Diff: https://dart.googlesource.com/pub.git/+/c4e9ddc888c3aa89ef4462f0c4298929191e32b9~..9bf4289d6fd5d6872a8929d6312bbd7098f3ea9c/
Change-Id: I5313721779a5a2cda825316a8ece6ed0cd57260e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/248803
Reviewed-by: Jonas Jensen <jonasfj@google.com>
Auto-Submit: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Sarah Zakarias <zarah@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
copybara-service bot pushed a commit that referenced this issue Jul 27, 2023
…st, vector_math.

Revisions updated by `dart tools/rev_sdk_deps.dart`.

collection (https://github.com/dart-lang/collection/compare/db343da..0a2885a):
  0a2885a  2023-07-25  Devon Carew  prep for publishing 1.18.0 (#299)

dartdoc (https://github.com/dart-lang/dartdoc/compare/a04ac3e..1cf8870):
  1cf88707  2023-07-26  Sam Rawlins  Convert 'p' prefixes to 'path' in tool/ (#3472)
  d44c8056  2023-07-26  Sam Rawlins  Move a few more grinder tasks to package:args commands (#3468)
  f66eb72d  2023-07-26  Sam Rawlins  Use path as import prefix in lib/ and test/ (#3471)
  34441f21  2023-07-25  Sam Rawlins  Move flutter-doc tasks to package:args; remove unused grinder tasks (#3466)

ecosystem (https://github.com/dart-lang/ecosystem/compare/27ff3e9..97fc1a7):
  97fc1a7  2023-07-25  Moritz  Fix comment posting from forks (#144)

test (https://github.com/dart-lang/test/compare/37e54e3..7f81dee):
  7f81deea  2023-07-24  Nate Bosch  Drop the Condition abstraction (#1956)

vector_math (https://github.com/google/vector_math.dart/compare/048777a..88bada3):
  88bada3  2023-07-26  John McCutchan  Revert "Fix rotation around Y axis (#262)" (#300)

Change-Id: Ib7bc8c1bab60450e6b328c3075207adef4cf642b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316621
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

2 participants