radianttap / swift-essentials Goto Github PK

I have just implemented your AsyncOperation in a project and it worked like a charm - until I started to use the new .progress property of OperationQueue (introduced with iOS 13).

I found out the hard way that you need to increase the .totalUnitCount of OperationQueue manually (https://stackoverflow.com/questions/60741067/progress-property-of-operationqueue-not-working-in-ios-13) when adding operations to the queue. But with this progress reporting started to work well - except for subclasses of AsyncOperation.

The .finished() method does get called on all subclasses of AsyncOperation, but somehow the .progress of the OperationQueue does not get the information about the finished task. The .operationCount of the queue decreases and the operations are removed from the [operations]. So in general AsyncOperation works. But the info about a finished operation is not signaled to the queue in a way that it can reflect this in its .progress property.

I am a bit at a loss here myself as I have no idea how iOS monitors this, so I really don't expect a quick fix. But I thought I'd let you know about this.

Thanks for your work!

I read your article about AsyncOperation.

I believe your implementation isn't 100 % correct because the state property — and by extension the isExecuting, isReady, and isFinished properties — aren't thread-safe.

The documentation for Operation states:

When you subclass NSOperation, you must make sure that any overridden methods remain safe to call from multiple threads.

This isn't just a theoretical concern. I used to have a very similar implementation at first and when I ran it the thread sanitizer diagnosed tons of data races. You should protect all accesses to the state property behind a mutex, dispatch queue, or similar.

@aodhol Please re-check with the latest version of AsyncOperation, if you still have that.
I completely changed internals, it works fine in two of my projects when compiled using Xcode 11.4.

I'm still seeing occasional TSan issues..

Originally posted by @aodhol in #4 (comment)