Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reentrancy anomaly detected when using KingfisherManager #20

Closed
PabloDomine opened this issue May 5, 2020 · 6 comments
Closed

Reentrancy anomaly detected when using KingfisherManager #20

PabloDomine opened this issue May 5, 2020 · 6 comments

Comments

@PabloDomine
Copy link

I'm trying to retrieve an image using the KingfisherManager like so:

func getImage(with path: String) -> Single<UIImage> {
        guard let url = URL(string: NetworkManager.getBaseUrl()) else {
            return Single.error(BaseError.doNothing)
        }
        let imageUrl = url.appendingPathComponent(path)
        
        guard let authToken = baseRepository.getLocalManager().getSessionData() else {
            return Single.error(BaseError.noSession)
        }
        
        let modifier = AnyModifier { request in
            var r = request
            r.setValue("Bearer \(authToken)", forHTTPHeaderField: "Authorization")
            return r
        }

        return KingfisherManager.shared.rx.retrieveImage(with: imageUrl,
                                                         options: [.requestModifier(modifier)])
    }

When the image is not valid, for example:

  • It's a svg image
  • token not sent

I get the following error:

⚠️ Reentrancy anomaly was detected.
  > Debugging: To debug this issue you can set a breakpoint in /[MyProject]/Pods/RxSwift/RxSwift/Rx.swift:96 and observe the call stack.
  > Problem: This behavior is breaking the observable sequence grammar. `next (error | completed)?`
    This behavior breaks the grammar because there is overlapping between sequence events.
    Observable sequence is trying to send an event before sending of previous event has finished.
  > Interpretation: This could mean that there is some kind of unexpected cyclic dependency in your code,
    or that the system is not behaving in the expected way.
  > Remedy: If this is the expected behavior this message can be suppressed by adding `.observeOn(MainScheduler.asyncInstance)`
    or by enqueuing sequence events in some other way.

With the following log, depending on which error I had. For example, in case it's a svg image:

Unhandled error happened: processorError(reason: Kingfisher.KingfisherError.ProcessorErrorReason.processingFailed(processor: Kingfisher.DefaultImageProcessor(identifier: ""), item: Kingfisher.ImageProcessItem.data(1368 bytes)))

I tried to silence the Reentrancy anomaly warning adding .observeOn(MainScheduler.asyncInstance) but it won't work.

Could this be in issue when detecting an error in the RxKingfisher library?

Thank you!

@freak4pc
Copy link
Member

freak4pc commented May 6, 2020

I think the issue isn't in the method, it's outside of it. Can you show the method that invoked getImage?

@PabloDomine
Copy link
Author

PabloDomine commented May 6, 2020

Hi @freak4pc , thanks for answering me back.

The code above, belongs to the ImageInteractor class, which handles the image requests.
I'm creating the observable, without subscribing, using the method getImage:

            if let imagePath = $0.categoryLink?.image {
                iconImage = imageInteractor.getImage(with: imagePath)
            }
            return IncidentViewModelItem(incidentId: $0.id,
                                         iconImage: iconImage,
                                         title: $0.name,
                                         stages: [])

And then, I'm setting up a tableview cell with the object IncidentViewModelItem I created.
In the tableview cell, I'm subscribing: (I put only the code that relates to Rx)

internal var disposeBag = DisposeBag()

override func prepareForReuse() {
        super.prepareForReuse()
        disposeBag = DisposeBag() 
}

func setup(viewModel: IncidentViewModelItem) {
        self.viewModel = viewModel
        
        viewModel.iconImage?
            .subscribeOn(ConcurrentDispatchQueueScheduler(qos: .background))
            .observeOn(MainScheduler.asyncInstance)
            .subscribe(onSuccess: { [weak self] (image) in
                self?.iconImageView.image = image
            }).disposed(by: disposeBag)
}

@freak4pc
Copy link
Member

freak4pc commented May 9, 2020

Unfortunately I don't have the full picture, yet.
But - the fact it only happens for token refreshals probably means the reentraancy is caused by the fact getImage invokes a token refresh which invokes getImage again, which is sort of a reentrancy, but it's really hard to say.

Regardless, this isn't an issue with RxKingfisher as far as I can tell since it works on the "happy path", so it's just some issue with scheduling / other code that you have there which I can't see

@eliotfowler
Copy link

eliotfowler commented May 17, 2020

I'm not using this library but have an ~identical implementation wrapping KingfisherManager and downloading an image. I found that the task getting cancelled causes a re-entrancy anomaly. I've debugged a few re-entrancy anomalies before but I'm struggling to figure out why cancelling the task does it here. I'm not sure if it's the same thing @PabloDomine is experiencing but figured I'd add my info here just in case it helps.

Commenting out the task?.cancel() in the Disposables.create { } block fixes the issue, but is not ideal because then the resource is retrieved even when the subscription has been disposed.

In the stack, I notice that the error block had been called several frames before task.cancel() gets called, which must be what's causing the unwanted cycle.

@eliotfowler
Copy link

Ah okay, I think I figured out what's going on here. If a task is cancelled, it appears that the error block is called twice in the same run loop – once for the request timing out, and once for the task being cancelled. I think the right approach here to only send error states when the error is not the task being cancelled (for this use case I don't think we would ever want that.)

I don't think there are any downsides to filtering out task cancelled errors, but maybe someone can back me up on that. I'll put up a quick PR with a fix and we can use that to have further discussion if that's helpful!

@PabloDomine
Copy link
Author

Thank you for stepping in @eliotfowler and sorry for the radio silence.
Unfortunately, I cannot test this issue under the same exact conditions anymore, since the API has changed and it's not longer returning vectorial images, but regular .jpg's.
If I have time, I will try to try this again in a different environment.
Thank you again @freak4pc and @eliotfowler!

@freak4pc freak4pc closed this as completed Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants