Moya icon indicating copy to clipboard operation
Moya copied to clipboard

trackInflights isn't thread-safe

Open jfrodden opened this issue 2 years ago • 1 comments

Moya -> 15.0.0

Different callbacks for the same service call don't always get called when we have the trackInflights set to true. It appears as though the @Atomic property wrapper around the MoyaProvider.internalInflightRequests property only ensures atomicity of reads/writes with respect to each read or write operation, but not across combined read/write operations, so the following code from MoyaProvider.requestNormal is not atomic across the read and write as it needs to be:

if trackInflights {
    var inflightCompletionBlocks = self.inflightRequests[endpoint] // <--- this is atomic
    inflightCompletionBlocks?.append(pluginsWithCompletion)
    self.internalInflightRequests[endpoint] = inflightCompletionBlocks // <--- and this is atomic, but not with the read

    if inflightCompletionBlocks != nil {
        return cancellableToken
    } else {
        self.internalInflightRequests[endpoint] = [pluginsWithCompletion] // <--- and this is atomic, but not with the read
    }
}

and later in the same method:

if self.trackInflights {
    self.inflightRequests[endpoint]?.forEach { $0(result) } // <--- this is atomic
    self.internalInflightRequests.removeValue(forKey: endpoint) // <--- and this is atomic, but not with the read
} else ...

I think a lock on self.inflightRequests is needed for each of the if trackInflights codepaths

jfrodden avatar Nov 10 '22 18:11 jfrodden

it looks like PR#2242 addresses exactly this issue#1085

jfrodden avatar Nov 10 '22 19:11 jfrodden