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

empterpreter async / emscripten_sleep() not working, no error #3307

Closed
rodneyrehm opened this issue Apr 4, 2015 · 21 comments
Closed

empterpreter async / emscripten_sleep() not working, no error #3307

rodneyrehm opened this issue Apr 4, 2015 · 21 comments

Comments

@rodneyrehm
Copy link

Hey,

I've been trying to get Empterpreter Async to work for sass.js. By performing the dynamic decision process I created a whitelist. Compiling with various debug flags works fine. But when running the built result in my browser I get garbage output, do not see the back from timeout log and don't get any error thrown from emscripten either.

The invocation chain is Module.ccall (JS) -> sass_compile_emscripten (C) -> libsass C++ stuff -> sass_importer_emscripten (C), the last function in turn invokes JS callbacks using extern "C" and --js-library

Since I did not change how data is passed, I expected everything to work as before, only with 1s delay because of the emscripten_sleep(1000). What piece of the puzzle am I missing here?


Steps to reproduce:

# download sass.js repository
git clone git@github.com:medialize/sass.js.git sass.js
cd sass.js
git checkout libsass/3.2-importer-async
# download libsass repository
(cd libsass && /bin/bash ./prepare.sh master)
# patch and compile libsass
(cd libsass && /bin/bash ./build.sh master debug)
# open interface in browser
open sass.source.html
# run example code outlined in sass.source.html
@kripken
Copy link
Member

kripken commented Apr 4, 2015

Does building with assertions and/or safe heap show anything?

Is this on latest incoming? Various bugs were fixed recently.

@rodneyrehm
Copy link
Author

Does building with assertions and/or safe heap show anything?

nope, that's why I felt like hitting the brick wall and opening an issue…

The steps to reproduce build with ASSERTIONS=1 and SAFE_HEAP=1.

Is this on latest incoming? Various bugs were fixed recently.

I just updated to 1.30.2 prior to this test (via brew install emscripten)


The only thing I have to offer is building a minimal test case for

  1. JS calling a C function
  2. calling a C function
  3. calling a JS function
  4. emscripten_sleep()
  5. calling a JS function
  6. return

I have no clue of your test infrastructure though…

@kripken
Copy link
Member

kripken commented Apr 4, 2015

A single standalone C++ file for a testcase would be good.

@rodneyrehm
Copy link
Author

I'm not able to reproduce the problem in a minimal test case. I've tried reproducing my scenario, but here I actually get an error thrown.

kripken added a commit that referenced this issue Apr 6, 2015
@kripken
Copy link
Member

kripken commented Apr 6, 2015

Ok, following your steps, I do get an abort. Not sure why you aren't seeing it - maybe change the abort method to have an alert, so it pauses there, and is more easily noticed?

The abort is because we use stackTrace in the emterpreter debug code, which can lead to a call to malloc... which is bad. Fixed on incoming.

With that out of the way, I get an assertion failure on the stack being adjusted during the async operation. I suspect this is because exceptions are on the stack when the pause occurs, which we have issues with.

Do you need exceptions here?

@rodneyrehm
Copy link
Author

Do you need exceptions here?

uncertain, but possible. libsass uses exceptions internally, I have no idea what the internal state looks like at the time of the callback being run (although i have no idea why there would be an exception on the stack).

You said there are known issues with exceptions. Are those planned to be tackled or ignored? I'm not exactly in a hurry to get this working, so I don't mind waiting…

My smaller test case, that produced the abort error (that libsass didn't) failed because of unclear reasons. I assume that was the stackTrace issue? That thing should not have failed, and it does not use exceptions…

@kripken
Copy link
Member

kripken commented Apr 6, 2015

Actually, looking a little closer, it isn't due to exceptions. It's the usage of ccall. ccall assumes synchronous execution, you can write

  var x = Module.ccall('myfunc');

and x should have the result. But that can't work with asynchronous execution.

Do you need the return value? If not, we could have a version of ccall that is aware of async. However, it would still be confusing if one did

  alert('pre');
  var x = Module.ccall('myfunc');
  alert('post');

the post will show up when the async operation begins. We can't really handle such things.

@rodneyrehm
Copy link
Author

Do you need the return value?

Well, I need a return value. How I get it is not important. In my libsass wrapper I pass various pointers to ccall() that are then "filled" by the function. If that still works, I can pass back the actual "return value" via one of those pointers as well.

If not, we could have a version of ccall that is aware of async.

We need some way to get to the data. Does that mean I have to kick off the async operation in one ccall() and poll for success (reading a pointer or using another ccall() in a loop)? If ccall() can't do the asnyc thing, but a new function could help with the polling, sure! (also a note in Empterpreter docs?)

kripken added a commit that referenced this issue Apr 7, 2015
@kripken
Copy link
Member

kripken commented Apr 7, 2015

Ok, the model I think will work best is now on incoming. You can do as async ccall, supplying a final argument to ccall { async: true }. In that case, there is no return value from the method being called, but instead a function, which you should call when the ccall completes later asynchronously. Example is in the test.

To get a return value, you can can EM_ASM from the C code, for example, as in that test.

Is that useful?

@rodneyrehm
Copy link
Author

In that case, there is no return value from the method being called, but instead a function, which you should call when the ccall completes later asynchronously.

I'm a bit confused by how this is supposed to work. I'll try to explain what I understood I have to do in order to accomplish my goals (allow a sync C function to run async JS callbacks) in code:

function runAsynFunction(done) {
  // prepare flag that we will set in library.js once the c function is done
  window._async_function_done = false;
  // invoke the C function, make emscripten trigger the async behavior
  var cleanupAsyncCcall = Module.ccall(
    'async_function',
    null,
    ['string'],
    ['some input string'],
    {async: true}
  );
  // ccall() returns immediately, although it's actually still running
  function isAsyncFunctionReturned() {
    // check if the c function is executed
    if (!window._async_function_done) {
      // wait and repeat
      return setTimeout(isAsyncFunctionReturned, 100);
    }
    // we're done, so cleanup and run the callback (resolve the promise, …)
    cleanupAsyncCcall();
    done(window._async_function_return_value);
  }
  isAsyncFunctionReturned();
}
// defined in library.js
extern "C" {
  void doAsyncCallback(char *input);
  bool isAsyncCallbackDone();
  void returnAsyncFunction(char *message);
}
// exported function
void async_function(char *input) {
  // do some sync stuff
  doAsyncCallback();
  while (!isAsyncCallbackDone()) {
    // wait, then recheck
    emscripten_sleep(100);
  }
  returnAsyncFunction(strdup("passing back return value"));
}
// library.js
var asyncCallbackDone = false;
// extern C to kick off async callback
function doAsyncCallback(inputPointer) {
  // make sure isAsyncCallbackDone() "blocks" until we're ready
  asyncCallbackDone = false;
  // work the input
  var input = Module.Pointer_stringify(inputPointer);
  // kick off our async thing and wait
  doSomethingAsync(input).then(function() {
    // async operation completed, set flag to inform C
    asyncCallbackDone = true;
  })
}
// extern C to query state of async callback
function isAsyncCallbackDone() {
  return Number(asyncCallbackDone);
}
// extern C to inform JS about function execution completion
function returnAsyncFunction(returnValuePointer) {
  var returnValue = Module.Pointer_stringify(returnValuePointer);
  window._async_function_done = true;
  window._async_function_return_value = returnValue;
}

To get a return value, you can can EM_ASM from the C code, for example, as in that test.

I don't think I understand what you're trying to say :/

Is that useful?

It is useful on a level that allows me to solve the problem. If we're talking "simple, obvious and easy to use", I guess I still have a few ideas…

  • The timeout-loop in runAsynFunction() to eventually resolve the promise (or run the callback) run the cleanup function returned by Module.ccall() seems rather generic. I'm sure this could be nicely wrapped into Module.ccallAsync(name, returnValueType, argumentTypes, argumentValues, callbackFunction). That can, of course, only work if the extern C returnAsyncFunction() is part of the empterpreter async package.
  • Module.ccall() returning immediately with the c function in an unknown state is confusing, if {async: true} is all it takes to trigger that behavior.
  • The doAsyncCallback(); while(!isAsyncCallbackDone()){ emscripten_sleep(100); } thing also looks like a generic enough pattern to optimize to something like emscripten_wait_for(doAsyncCallback()) (maybe not exactly that…)

@kripken
Copy link
Member

kripken commented Apr 8, 2015

I'm not sure I followed that example. But I see that the need to manually clean up is confusing, and meanwhile I figured out a way to avoid that. So now the model is simpler. Here is the testcase in the test suite, to explain:

// C function
void func() {
  printf("Hello\n");
  emscripten_sleep(1000);
  printf("World\n");
  EM_ASM( finished() );
}

That C function prints, waits a second asynchronously, then prints some more Then it calls finished in JS to signal that the async operation is complete. It could also return a value there if you want. This would then be called by something like

function finished() {
  console.log('the async operation is done!');
}
ccall('func', null, [], [], { async: true });

The ccall will return when the async operation begins. It is still in the middle of executing func at that stage; we need to wait for more turns of the event loop. Later, finish will be called when it is done.

@rodneyrehm
Copy link
Author

I'm not sure I followed that example

I was trying to illustrate how the manual polling/waiting for function completion worked, but see how ridiculously convoluted that was.

I now understand what that EM_ASM does. When looking at calling JS from C I focused on the library thing probably because I needed to pass strings, which EM_ASM* doesn't seem to support.


I guess that EM_ASM( finished() ); is executed synchronously. How would I go about cleaning up memory/stack here? Simply use setImmediate() on the stackRestore()?

var _stack = Module.Runtime.stackSave();
function finished() {
  console.log('the async operation is done!');
  Module.Runtime.stackRestore(_stack);
}
ccall('func', null, [], [], { async: true });

Thanks for this! any idea when this will be officially released?

@kripken
Copy link
Member

kripken commented Apr 8, 2015

You can pass strings, but you will need to do a little manual work. A C string is just a pointer, to turn that into a JS string, call Pointer_stringify.

Yes, EM_ASM is synchronous.

What memory/stack do you need to clean up? ccall will now clean its own stack usage up. Otherwise, if you have your own stack usage you perform before ccall, you can clean it up in that finished method, as you wrote there.

Not sure when this will reach a stable release. If the bots are stable early next week, perhaps.

@rodneyrehm
Copy link
Author

You can pass strings, but you will need to do a little manual work. A C string is just a pointer, to turn that into a JS string, call Pointer_stringify.

that I'm aware of. However don't see how I would pass that to EM_ASM, unless the following works?

char *text = strdup("foo");
EM_ASM_INT({
  var text = Module.Pointer_stringify($0);
  console.log("message", text);
}, text);

What memory/stack do you need to clean up?

I create a bunch of pointers using Module.allocate([0], 'i8', ALLOC_STACK); to pass as argument to ccall, so the c function can fill that pointer with another pointer to the actual data. discussed here.


Update: confirming that this works perfectly fine!

@kripken
Copy link
Member

kripken commented Apr 8, 2015

Yeah, that looks like it should work.

@rodneyrehm
Copy link
Author

I guess this issue can be closed now?

@kripken
Copy link
Member

kripken commented Apr 9, 2015

Sure.

@kripken kripken closed this as completed Apr 9, 2015
@rodneyrehm
Copy link
Author

totally forogt: THANK YOU! :)

@kripken
Copy link
Member

kripken commented Apr 9, 2015

np, and thanks for filing - this isn't a much used feature yet, so it's great to get feedback and work out the issues.

@rodneyrehm
Copy link
Author

I just installed 1.30.5 manually and I'm able to confirm success! yay! :)

@kripken
Copy link
Member

kripken commented Apr 13, 2015

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants