emscripten-core / emscripten-core/emscripten

Improve abort messages over incorrect std::thread::~thread() invocation and std::terminate()?

Open
#17,933 1 comment 1 reaction 0 assignees View on GitHub
Dominant language
C++
Stars
27.6k
Forks
3.6k
Avg merge
1d 1h
Merged PRs (30d)
105

Description

We recently had a customer surprised about the following (incorrect) C++ code:

`a.cpp`
```cpp
#include

int main()
{
std::thread thread([&]() {
printf("Hello!\n");
});
}
```
```
em++ a.cpp -o a.html -pthread -g3
```
The above code crashes with the error
![image](https://user-images.githubusercontent.com/225351/192751141-30ac71f9-e0bb-47bc-b76f-45b633fb87de.png)

Which does not quite help the developer figure out what they are doing wrong.

The reason for this crash is that letting an unjoined joinable/nondetached thread be destroyed is against the contract of `std::thread`: https://en.cppreference.com/w/cpp/thread/thread/~thread

which states:

*"If this has an associated thread (joinable() == true), [std::terminate](http://en.cppreference.com/w/cpp/error/terminate)() is called."*

The developer is supposed to explicitly tell the API whether they want to detach the thread (make it "fire-and-forget"), or to conclude its lifetime by joining it (getting a return value). Letting go of a thread object without choosing one of these behaviors is wrong, hence the abort.

However we don't do a great job helping developers figure this out.

Looking at

https://github.com/emscripten-core/emscripten/blob/fc3a21739ba9720b9a1010bc771ad2d16d73fee5/system/lib/libcxx/src/thread.cpp#L36-L40

and

https://github.com/emscripten-core/emscripten/blob/fc3a21739ba9720b9a1010bc771ad2d16d73fee5/system/lib/libcxxabi/src/cxa_default_handlers.cpp#L95

leaves something to be desired in developer friendliness of both libcxx and libcxxabi. It would be great in development builds to abort execution with a proper readable error

```
std::thread::~thread(): calling std::terminate() due to destruction of an unjoined joinable thread. Either .join() or .detach() the thread before deleting it.
```

Also, looking at the behavior of `std::terminate()`, it would also be great if calls to `std::terminate()` would abort execution with an error message

```
Aborted(native code called std::terminate())
```

These are libcxx and libcxxabi side issues, which makes me sad that I can't just improve this. Any chance we'd want to accommodate a localmod?

There are two reasons why I think Emscripten would want to locally change this:
- we rely much more on the error messages compared to native code world, since there are no functioning debuggers for Wasm that would work in scale, and
- multithreading in WebAssembly behaves a bit different from other platforms that developers are expected to run into troubleshooting scenarios when targeting SharedArrayBuffer, so having clearer error messages would help there.

Or do we know people who maintain libcxx and libcxxabi if they might be willing to improve this upstream?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.