Address thread safety issues.

[freakboy3742]

This PR addresses a number of thread safety issues with Rubicon, identified as a result of the Toga GUI testbed app.

1. CFEventLoop was always binding to the main thread's event loop. Event loops are thread specific; in some cases, this could lead to a segfault on shutdown, as the cleanup process would attempt to register and close the self-socket, but either the main event loop had been closed and disposed of, or the self-socket was no longer valid. (Segmentation fault when exiting multi-threaded program #250)

2. The call to get a run loop is declared as following the GET convention; without explicit retention, it is possible for the event loop to be disposed of before CFEventLoop has finished with it. This is unlikely to occur in practice, but can't hurt to protect against.

3. Calls to CFEventLoop.stop() weren't invoking logic on the parent; as a result, the _stopping flag wouldn't be set.

4. If a socket was disabled and the event loop was in the process of closing and an event occurred, it was possible for a run loop to be removed twice.

5. If a memory address was used by the Objective C runtime, wrapped in a ObjCInstance object, then disposed of by Objective C, there is a window between that disposal and the removal of the object from the ObjCInstance instance cache. As a result, if the memory address is re-used, a stale wrapper would be returned. That wrapper could (and usually would) have a different type to the new object using the address, causing errors because the "new" object will report as being of a different type, and respond to different methods. (Attribute intermittently returns the wrong object #249)

6. In a multithreaded setup, when creating a new ObjCInstance, it was possible to create 2 different ObjCInstance wrappers for the same memory address; one of those wrappers would be lost from the instance cache (as the ObjC memory address is used as a key). (Race condition when instantiating ObjCInstance objects #251)

7. In a multithreaded setup, it was possible for the method cache to become corrupted if two separate threads attempt to access a method or property on an object at the same time. One of the attempts would discover an empty cache, and attempt to populate that cache; the second attempt would see an apparently populated cache, and look up a method or property. If the first thread had not yet populated the cache for the requested method/attribute, it would report an AttributeError, even though the attribute should exist. (Race condition when populating the ObjCClass method/attribute cache #252).

There are tests for 5-7; however, I can't work out how to write tests that will reliably exercise 1-4 to occur.

Fixes #249
Fixes #250
Fixes #251
Fixes #252

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[dgelessus]

Oof, this looks like a really hairy set of problems. Thank you very much for debugging and fixing everything!

It's unfortunate that the fix makes ObjCInstance.__new__ even more complex than it already is, but it's definitely needed. I thought about this a bit and can't think of a way to simplify this without also breaking the fix.

It might be possible to optimize some of the locking code in some way and perhaps avoid locking entirely in some cases, but it's probably not worth worrying about that unless the locking causes significant performance problems.

There's a somewhat relevant comment in the implementation of ObjCInstance.objc_class about objects changing their class after creation. That probably needs updating now - even though this fix isn't about objects actually changing their class.

[freakboy3742]

Oof, this looks like a really hairy set of problems. Thank you very much for debugging and fixing everything!

It's unfortunate that the fix makes ObjCInstance.__new__ even more complex than it already is, but it's definitely needed. I thought about this a bit and can't think of a way to simplify this without also breaking the fix.

It might be possible to optimize some of the locking code in some way and perhaps avoid locking entirely in some cases, but it's probably not worth worrying about that unless the locking causes significant performance problems.

I had exactly the same thought. I tried to make the locks as granular as possible while keeping the locking behavior obvious enough that it's possible to reason that deadlocks won't happen, and without needing to create a lock on each individual attribute. A bunch of the locks around attribute population are possibly non-required, as they are populated on an attribute-by-attribute basis; and the cache-spoiling behavior will only lead to a single attribute accessor being created then overwritten. I guess that's a "memory usage at time of first access vs guaranteed single creation" tradeoff; time will tell whether the performance overhead is enough of a problem to warrant revisiting this.

There's a somewhat relevant comment in the implementation of ObjCInstance.objc_class about objects changing their class after creation. That probably needs updating now - even though this fix isn't about objects actually changing their class.

I'll make that update before I merge.