Added --thread_safe option to webidl_binder.py

[jrouwe]

This emits all temporaries as 'static thread_local temp;' instead of 'static temp;' and prevents race conditions when you use the resulting library in a multithreaded environment.

E.g. if you create an IDL file like:

interface Vec3 {
	[Const, Value] Vec3 Cross([Const, Ref] Vec3 inRHS);
};

Previously the generated cpp file would look like:

const Vec3* EMSCRIPTEN_KEEPALIVE emscripten_bind_Vec3_Cross_1(Vec3* self, const Vec3* inRHS) {
  static Vec3 temp;
  return (temp = self->Cross(*inRHS), &temp);
}

Which has a race condition when you call into the Cross function from multiple JavaScript threads as they all use the same temp variable.

With this new parameter the resulting code will look like:

const Vec3* EMSCRIPTEN_KEEPALIVE emscripten_bind_Vec3_Cross_1(Vec3* self, const Vec3* inRHS) {
  static thread_local Vec3 temp;
  return (temp = self->Cross(*inRHS), &temp);
}

Which guarantees that no other threads overwrite temp.

[sbc100]

can you use dashes in the option name (e.g. "--thread-safe" )

[jrouwe]

Fixed.

[sbc100]

Maybe we should just always emit this, when TLS gets lowered away when not building with thread support?

[jrouwe]

We could, and you're right that if you build without -pthread the resulting code is the same:

https://godbolt.org/z/6cE6T8a47

When adding -pthread there is an extra instruction which adds a bit of overhead:

I cannot predict if people are ok with this overhead. Their library may use multiple threads internally, but maybe they don't interact with the library from multiple threads. In that case there's no benefit in adding the thread_local attribute.

[sbc100]

I think if you are doing a pthread build we should just default to being thread safe.

[jrouwe]

Ok, in that case the change becomes really simple. I've pushed the new code.

[kripken]

Please add a comment, maybe something like "Avoid sharing this static temp var between threads, which could race." Otherwise looks great.

[jrouwe]

Done.