Adding the ability to post an error on any object

[mattkae]

What's new?

• C++ classes are now initialized with their protocol ID during the associate step
• We keep a map from protocol_id -> Resource in a thread-local storage
• The ProtocolError now includes this protocol ID when it sends exceptions
• The ID gets parsed, looked up in the map, and then used to post an error if it is available

Checklist

• Tests added and pass
• Adequate documentation added
• (optional) Added Screenshots or videos

[RAOF]

Wheeee! What a mess. I've suggested what I think is an improvement; please check if that works!

[RAOF]

I'm not wild about making this a thread-local; this makes our C++ side no longer threadsafe.

A grungy, but possible, alternative would be to use handle.object_for_protocol_id()? This would require stashing the &'static protocol::Interface in addition to the object id in the C++ side, but because it's 'static that would be safe (although it's marshalling pointer -> string -> pointer, so looks groody)

[mattkae]

My push back is that:

1. It adds a lot of complication from what I've seen so far
2. When else would you be throwing a protocol error outside of the Wayland thread? Errors are thrown in response to some request typically, not out of the blue AFAIK

[mattkae]

What if we went the easy route and put the map behind a lock. That way we can avoid unnecessary thread safe code, and the lookup table will always be thread safe. And therefore we can post errors from any thread 😄 This seems like the best route in my opinion as it avoid a lot of unsafety and complication (e.g. serializing pointers)

[RAOF]

Yeah, that works, but...

One of the things that passing the &'static Interface would have handled (that I didn't explicitly mention) is removing things from the map; as it is now, IDs are never removed for dead objects.

Can we also remove from the map when the object dies? Otherwise clients will be able to cause us to consume unbounded memory 😮‍💨

[mattkae]

Yeah, that works, but...

One of the things that passing the &'static Interface would have handled (that I didn't explicitly mention) is removing things from the map; as it is now, IDs are never removed for dead objects.

Can we also remove from the map when the object dies? Otherwise clients will be able to cause us to consume unbounded memory 😮‍💨

Oof of course!

Luckily, our Dispatch provides a destroyed call, so we can just remove ourselves from the map there :)

[RAOF]

Sorry to make one last request; feel free to ignore it or to apply that set of changes and merge.

[RAOF]

Suggested change

	OBJECT_REGISTRY.write().unwrap_or_else(|e| e.into_inner()).insert(id.protocol_id(), id);
	OBJECT_REGISTRY.write().expect("lock poisoned").insert(id.protocol_id(), id);

I don't know if we can witness it, but if the Mutex is poisoned then we probably want to bail rather than continue with weirdness.