paullouisageneau / datachannel-wasm

C++ WebRTC Data Channels and WebSockets for WebAssembly in browsers
MIT License
148 stars 25 forks source link

Concerns about Async operations #19

Closed jeffRTC closed 3 years ago

jeffRTC commented 3 years ago

I see a lot of issues lately and I probably think your events causing this. As per emscripten docs,

While waiting on an asynchronous operation browser events can happen. That is often the point of using Asyncify, but unexpected events can happen too. For example, if you just want to pause for 100ms then you can call emscripten_sleep(100), but if you have any event listeners, say for a keypress, then if a key is pressed the handler will fire. If that handler calls into compiled code, then it can be confusing, since it starts to look like coroutines or multithreading, with multiple executions interleaved.

It is not safe to start an async operation while another is already running. The first must complete before the second begins.

Such interleaving may also break assumptions in your codebase. For example, if a function uses a global and assumes nothing else can modify it until it returns, but if that function sleeps and an event causes other code to change that global, then bad things can happen.

My application too heavy on Asyncify. I just wondering what are your thoughts on this?

For example,

If you call this function 1000 times from JS side, the said behavior happen..

void foo(emscripten::val data) {
        emscripten_sleep(100);
}
jeffRTC commented 3 years ago
RTCclient.wasm:0x76f28 Uncaught RuntimeError: memory access out of bounds
    at pb (wasm-function[1482]:0x76f28)
    at Object.ret.<computed> [as dynCall_vi] (runtime.js:16:44665)
    at RTCDataChannel.cb (runtime.js:16:111358)
paullouisageneau commented 3 years ago

My application too heavy on Asyncify. I just wondering what are your thoughts on this?

As the doc says, asyncify coexisting with callbacks should not be a problem but you have to carefully watch for possible race conditions.

In particular, the doc says "It is not safe to start an async operation while another is already running", so for instance if you call emscripten_sleep() and a callback is triggered while sleeping, if you call emscripten_sleep() in the callback, it will crash.

RTCclient.wasm:0x76f28 Uncaught RuntimeError: memory access out of bounds
    at pb (wasm-function[1482]:0x76f28)
    at Object.ret.<computed> [as dynCall_vi] (runtime.js:16:44665)
    at RTCDataChannel.cb (runtime.js:16:111358)

This looks like a segmentation fault in wasm code triggered by a callback. You should try building with the address sanitizer.

jeffRTC commented 3 years ago

so for instance if you call emscripten_sleep() and a callback is triggered while sleeping, if you call emscripten_sleep() in the callback, it will crash.

That's what actually happen here. A callback receives while it sleeping and it crash.

But if I remove the async stuff from code, it doesn't crash..

I'm am about to move the async code as a JS library like datachannel-wasm done

jeffRTC commented 3 years ago

Lib is right way to do this. I copied pasted your code..

Closing...