We cannot run Swindler tests in our serial queue and have them "block" on waiting for sub-tasks to run in the same queue (because they will be blocking those sub-tasks from running). Unfortunately, there is no reentrant queue in GCD, and since PromiseKit is hardwired to use dispatch queues we are pretty much locked in to supporting these.
One alternative is to use locks, but I'm not very comfortable with this idea because it's very easy to miss a spot where I needed to add a lock and forgot. There are a number of "ways in" to the Swindler code (to name a few: public API (which is easy to cover), notifications from the NotificationCenter, notifications from AXSwift observers, promise callbacks, and property event handlers). I also don't know of an easy way to assert that we have a lock on this thread somewhere. Maybe we can use a custom lock class with a thread-local, or actually pass a lock guard around as proof? TODO: investigate the thread-local idea
What I'd rather do is put everything on one serial queue, and assert that we are on that queue in a number of key places throughout the code. This leads to the above problem with testing. However, there might be a way around this:
In tests, set up our serial queue with the main queue as its target queue. This means we will not support dispatching sync tasks on our swindler queue from the main queue. In the public API, have a compile-time switch that disables the queue precondition check and replaces it with (check we're on main thread, then set thread-local to assert we've gotten here via the public API). Within Swindler itself, have the assertion be (check that EITHER the public-API-thread-local is set, OR we're currently running on our Swindler queue). This should catch code that tries to access Swindler in a thread-unsafe way. In production, turn off this compile-time switch and have the public API assert that we are being called from our queue.
Again, this will break if there is any code doing a sync call on our Swindler thread from within the test, but we can choose not to support that. We could make it harder to do this by not exposing the Swindler queue in our public API (though there would still be a way to get to it, via the PromiseKit API).
What about tests of applications/libraries that use Swindler? Well, if those applications use Swindler off the main thread, then they are already written in such a way that they don't need direct access; but this may be just around the edges. A test might indeed want to call into a method that assumes that it is already on the Swindler queue. Perhaps for those we can support a test-only mode that delegates to our Swindler queue (off main thread) in a synchronous fashion.
It may be possible to do this for Swindler tests as well. Is there a reason I didn't think of this before? Ah, right, because the GCD queue API explicitly prevents us from doing anything reentrant. Event handler code will end up running on the Swindler queue (or should it?), then it will try to access the Swindler object, which will deadlock trying to run on the current queue. Perhaps we can detect only main-thread accesses and redirect those; all others will assert that we're on the Swindler queue? This actually sounds promising.
Or, maybe the true solution is to rewrite all the Swindler tests to access Swindler (and the test driver objects it modifies) via the Swindler queue, and stop trying to hack around the inconvenience of this. It would probably be a lot simpler than most of these hacks.
Expanding on my idea to allow public API usage from the main thread in tests: promise callbacks will run on the Swindler thread, so we still need to allow public API usage from that thread.
My problem with even this solution is that we're relying on assertions to verify program correctness. If an application has a rarely-used path that it forgets to schedule on the correct queue, it will crash on an end user's machine.
Perhaps there are (big) API changes that we can make to prevent this from happening, or at least make it very difficult to do accidentally. One such idea is to have a WindowRef class that supports equality, but only allow access to the Window itself from within event handlers, or through a special proxy method on the WindowRef class. It would still be possible to save off a reference to the Window, but this would be explicitly discouraged.
The other alternative is to simply make the API safe to use from other threads, or at the very least the main thread. This means either turning my test-only hack on all the time, or using locks directly. I really want to support performant code and not require it to re-acquire a lock for every iteration in a loop. (That'd be the motivation for using the method that only allows access from the Swindler thread or the main thread.)
OTOH, asserting about the current queue is a much better way of catching bugs than using locks and "hoping it all works". As long as an application has decent test coverage, and we give it a way to properly test itself, it shouldn't have these rarely used paths going untested. What about, for application tests, setting a per-test flag to allow Swindler access from the main thread? That way you can opt in for unit tests of something that interacts with Swindler objects directly. Or we can just require them to dispatch everything to the Swindler queue (including within expect()s... this is pretty inconvenient).
I actually don't think my hack is possible anymore after all. It's not really possible to tell if you're running on the main queue. The main thread is another story, but you could be executing on the Swindler queue inside the main thread. Maybe there's a way to disable that, but it would hurt performance more than using a lock.
So, it seems my options are using a reentrant lock (which hopefully is fast in the reentrant case, so app code can run a function that touches a bunch of things with the lock held), or simply forcing everything to go through the Swindler queue and rewriting some tests. I can maybe still support one of the above hacks for Swindler's own tests, but I can't really support them outside Swindler.
Thoughts on testing...
We cannot run Swindler tests in our serial queue and have them "block" on waiting for sub-tasks to run in the same queue (because they will be blocking those sub-tasks from running). Unfortunately, there is no reentrant queue in GCD, and since PromiseKit is hardwired to use dispatch queues we are pretty much locked in to supporting these.
One alternative is to use locks, but I'm not very comfortable with this idea because it's very easy to miss a spot where I needed to add a lock and forgot. There are a number of "ways in" to the Swindler code (to name a few: public API (which is easy to cover), notifications from the NotificationCenter, notifications from AXSwift observers, promise callbacks, and property event handlers). I also don't know of an easy way to assert that we have a lock on this thread somewhere. Maybe we can use a custom lock class with a thread-local, or actually pass a lock guard around as proof? TODO: investigate the thread-local idea
What I'd rather do is put everything on one serial queue, and assert that we are on that queue in a number of key places throughout the code. This leads to the above problem with testing. However, there might be a way around this:
In tests, set up our serial queue with the main queue as its target queue. This means we will not support dispatching sync tasks on our swindler queue from the main queue. In the public API, have a compile-time switch that disables the queue precondition check and replaces it with (check we're on main thread, then set thread-local to assert we've gotten here via the public API). Within Swindler itself, have the assertion be (check that EITHER the public-API-thread-local is set, OR we're currently running on our Swindler queue). This should catch code that tries to access Swindler in a thread-unsafe way. In production, turn off this compile-time switch and have the public API assert that we are being called from our queue.
Again, this will break if there is any code doing a sync call on our Swindler thread from within the test, but we can choose not to support that. We could make it harder to do this by not exposing the Swindler queue in our public API (though there would still be a way to get to it, via the PromiseKit API).
What about tests of applications/libraries that use Swindler? Well, if those applications use Swindler off the main thread, then they are already written in such a way that they don't need direct access; but this may be just around the edges. A test might indeed want to call into a method that assumes that it is already on the Swindler queue. Perhaps for those we can support a test-only mode that delegates to our Swindler queue (off main thread) in a synchronous fashion.
It may be possible to do this for Swindler tests as well. Is there a reason I didn't think of this before? Ah, right, because the GCD queue API explicitly prevents us from doing anything reentrant. Event handler code will end up running on the Swindler queue (or should it?), then it will try to access the Swindler object, which will deadlock trying to run on the current queue. Perhaps we can detect only main-thread accesses and redirect those; all others will assert that we're on the Swindler queue? This actually sounds promising.
Or, maybe the true solution is to rewrite all the Swindler tests to access Swindler (and the test driver objects it modifies) via the Swindler queue, and stop trying to hack around the inconvenience of this. It would probably be a lot simpler than most of these hacks.
Expanding on my idea to allow public API usage from the main thread in tests: promise callbacks will run on the Swindler thread, so we still need to allow public API usage from that thread.
My problem with even this solution is that we're relying on assertions to verify program correctness. If an application has a rarely-used path that it forgets to schedule on the correct queue, it will crash on an end user's machine.
Perhaps there are (big) API changes that we can make to prevent this from happening, or at least make it very difficult to do accidentally. One such idea is to have a
WindowRefclass that supports equality, but only allow access to theWindowitself from within event handlers, or through a special proxy method on theWindowRefclass. It would still be possible to save off a reference to theWindow, but this would be explicitly discouraged.The other alternative is to simply make the API safe to use from other threads, or at the very least the main thread. This means either turning my test-only hack on all the time, or using locks directly. I really want to support performant code and not require it to re-acquire a lock for every iteration in a loop. (That'd be the motivation for using the method that only allows access from the Swindler thread or the main thread.)
OTOH, asserting about the current queue is a much better way of catching bugs than using locks and "hoping it all works". As long as an application has decent test coverage, and we give it a way to properly test itself, it shouldn't have these rarely used paths going untested. What about, for application tests, setting a per-test flag to allow Swindler access from the main thread? That way you can opt in for unit tests of something that interacts with Swindler objects directly. Or we can just require them to dispatch everything to the Swindler queue (including within
expect()s... this is pretty inconvenient).I actually don't think my hack is possible anymore after all. It's not really possible to tell if you're running on the main queue. The main thread is another story, but you could be executing on the Swindler queue inside the main thread. Maybe there's a way to disable that, but it would hurt performance more than using a lock.
So, it seems my options are using a reentrant lock (which hopefully is fast in the reentrant case, so app code can run a function that touches a bunch of things with the lock held), or simply forcing everything to go through the Swindler queue and rewriting some tests. I can maybe still support one of the above hacks for Swindler's own tests, but I can't really support them outside Swindler.