search

shaka-project / shaka-player-embedded

Shaka Player in a C++ Framework
Apache License 2.0
239 stars 62 forks source link

Refactor XMLHttpRequest lock and thread handling #65

Open TheModMaker opened 4 years ago

TheModMaker commented 4 years ago

Currently, XMLHttpRequest doesn't handle locks and threads correctly. It is mainly used on the JS main thread, so we don't need locks for that; but it also is called from the NetworkThread for network events. This multi-thread usage requires locks. Currently, the background calls also modify fields that are directly exposed to JavaScript, and the mapping framework doesn't handle locks, so these can result in a data race.

Also, since XMLHttpRequest and NetworkThread call into each other, it might be possible to deadlock, if not handled correctly. This should be refactored to avoid this bidirectional calls and deadlock possibility.

The only time we need to interact with the background threads is to (a) fire JS events, or (b) send or receive data. The data handling could be done by NetworkThread instead. The event firing could be done by having NetworkThread schedule the callbacks to happen on the JS main thread instead. This allows XMLHttpRequest to only work on the JS main thread and avoids complex multi-thread work.

TheModMaker commented 4 years ago

There is another data race between the two for the CURL data. The NetworkThread reads the individual requests as part of curl_multi_perform, but XMLHttpRequest reads the request directly on other threads.

This should be easily detectable with Valgrind: valgrind --tool=helgrind out/Debug/demo.