* Use Schedulers for Promise methods
* Always put the scheduler argument first
* Fix ordering in calls
* its hard to write code in github workspaces...
Fixes a bug where some emoji were mistaken for
zalgo, causing all diacritics in a message to get
stripped. Instead, replace "risky" characters with
too many diacritics with the unicode replacement
character, leaving others alone.
Avoids a reentrancy deadlock when a promise's 'result' is used from
within an observer callback. This is a behavior change only in the
following scenario:
1. A promise is sealed and starts running its observers
2. While fulfilling observers, a new observer is added (on the same
thread or from on another thread)
Previously, the future would wait until all existing observers have
been run, then run the new observer. Now, the new observer will run
immediately, since the only thing being checked is that the promise
has been sealed. We could do more work to preserve the old behavior,
but it's better to not depend on such intricacies in the first place.
The issue:
`asyncIfNecessary` will always synchronously execute the provided
closure if the target queue and the current queue are the same.
Since promises chains can become extremely long, especially with any
sort of retry behavior, it's possible to exhaust the stack this way. The
overhead for each synchronous promise is ~2.5k of stack size per step.
The fix:
If we want to continue to use `asyncIfNecessary`, we should consider it
"necessary" to async if we're approaching the stack boundary of the
current thread.
This is a bit of a hack, but it seems to work in testing. It's also
reasonably safe. If this does introduce a bug, it'll just be that we're
too aggressive about asyncing. But asyncs are always correct,
synchronous execution of the promise closure is just a perf
optimization.
When we're calling dispatch_get_current_queue(), ARC automatically
retains the result of the function and increments the external reference
count of the resulting dispatch queue.
The problem is dispatch queues assume that once their external reference
count goes to zero that the queue is no longer in use. By
re-incrementing, we trigger an assert within libdispatch.
This fix applies a bandaid to this issue. If we don't store the result
of the function in an ARC pointer, then we're not going to inadvetently
retain a dead queue. Instead, we'll use pointer comparisons to check for
equality:
- Since we're not going to use the queue for anything or persist it,
there's no risk we accidentally extend the queue's lifetime in bad
ways.
- If the queue is about to expire, we're still allowed to continue our
block's work as long as we want.
I think it'd be great if we found an alternative to
dispatch_get_current_queue() that allows us to achieve the same
performance. I'll continue to think about this. In the meantime, this
appears to fix this crash.
* No longer maintain the last queue through the chain. Eventually we may want to
do this, but for now it's not safe to do until we audit all our queue usage.
* Dispatch to the main queue by default if no queue is specified, matching the
behavior of PromiseKit. Eventually, we may want to require an explicit queue
to be provided on every element of the chain to avoid lots of extraneous work
occuring on the main queue, but we'll need to do an audit of all our call
sites.
* Cleanup generic usage to mitigate problems where Value can be a Promise. These
are a tricky subset of issues that result in subtle bugs where, for example,
you can have "firstly" block that returns a promise, but doesn't actually run
the promise, instead it just returns the promise as a value to the next block
in the chain. It requires some very delicate massaging of generics to get this
to work properly implicitly. If only swift allowed "not" declerations for
generics, so we could say a promise could be anything *but* another Thenable
type.
