MoyaProvider gets deallocated when there're ongoing pending tasks.

Created on 27 Jan 2018  路  19Comments  路  Source: Moya/Moya

This is a discussion following the issue raised in #1554.

For users like me that are adopting the 10.X version of Moya, maybe have noted that, opposing to the old behavior found in Moya 8.X where the MoyaProvider was alive until all of its requests where finished. The user always was responsible of disposing any resources (tasks) that might be going on and not wanted to be finished anymore.

This approach of requests strongly referencing the MoyaProvider allowed developers to use elegant protocol extensions of generic services that generated computed providers that were alive all the lifetime of their fired requests.

As of 10.X this behavior is not happening anymore due to requests weakly reference their provider and thus, getting deallocated or even not fired when their provider gets deallocated. This pushes developers to maybe create hacky instances of MoyaProvider if wanted to use the old protocol extension approach. On the other hand, this frees the developer to get track of any resources but it gets a, IMHO, a weird behavior where requests are nor cancelled nor failed nor completed, just don't happen because their MoyaProvider is already deallocated, which, depending on the specific use-case, might be hard to track.

Referencing to standard existing approaches in networking, Alamofire, which this library depends on, always let the developer to track the lifetime of any request using Request methods (suspend, resume and cancel), so the developer has always the control of the individual request, independently of other dependencies it might have.
Moya, reporting high rapport with reactive programming (observer pattern), such as ReactiveSwift and RxSwift, where the lifetime of any "Observable" is always managed by the user or the own state of the Observable itself but always reporting its state to any listeners that might be waiting for data comming through it.

I open a discussion about recovering the behavior of Moya 8.X and unifying that behavior in future releases of Moya. I don't think this should be treated as a _breaking change_ because, from the developer's point of view, it should behave as it is now, but in specific cases developers might keep in mind that their requests are now not disappearing _magically_, but controlling their requests, cancelling them, disposing them, etc.

As a final note, I try to keep this as educational and instructive as possible for all of us and I'm open minded to learn from the Moya team and community, which I love and want to support in first person.

馃殌

discussion rxmoya

Most helpful comment

Are there any supports on me opening a PR changing the referencing cycle to a strong reference on the Reactive and Rx extensions of the library? 馃槃

All 19 comments

Thanks for taking the time to formalize your thoughts @minuscorp 馃憤 This is also related to #905 #1380.

I've formalized a somehow, IMHO, super hacky solution that could be applied to any observable in the RxSwift environment. It is the retaining<T>(_ value: T) operator. Not sure if this is the issue where I should introduce this solution, but maybe this could be an option to consider, letting in the hands of the developer to retain a certain object during the lifetime of an Observable:

private class RetainToken<E, V> : ObservableConvertibleType, Disposable {
    private let _source: Observable<E>
    private var _retain: V!

    init(source: Observable<E>, value: V) {
        _source = source
        _retain = value
    }

    func dispose() {
        self._retain = nil
    }

    func asObservable() -> Observable<E> {
        return _source
    }

    deinit {
        self._retain = nil
    }
}

public extension ObservableType {
    public func retaining<T>(_ value: T) -> Observable<E> {
        return Observable.using({
            return RetainToken(source: self.asObservable(), value: value)
        }) { (t: RetainToken<E, T>) -> Observable<E> in
            return t.asObservable()
        }
    }
}

public extension PrimitiveSequence where TraitType == SingleTrait {
    public func retaining<T>(_ value: T) -> Single<E> {
        return Single.using({
            return RetainToken(source: self.asObservable(), value: value)
        }, primitiveSequenceFactory: { (t: RetainToken<E, T>) -> Single<E> in
            return t.asObservable().asSingle()
        })
    }
}

The idea is mostly based on the ActivityIndicator proposed in the RxSwift's repo. This operator makes sure that the object is referenced by a strong reference during the whole lifetime of the Observable in which this is applied.

@minuscorp Thanks for taking the time to share what worked for you in order to help other Moya users in the future 馃挴 I agree that is kind of rough 馃槗

I'd like to invite the Moya community to go through this and maybe find a solution that fits everyone's needs. Keep up the great work!

I think the main issue is that when the Provider architecture was decided upon, they were planned to be used as long lived objects, that basically matched the lifetime of the application.

I think refactoring them to change that would be awesome, and adding a static func request that would return a disposable. That seems like the _right_ answer to me 馃檭

The only think I disagree is that how can be the Observable contract broken when wrapping the MoyaProvider, thing that would be easily patched by strongly referencing self in here

Are there any supports on me opening a PR changing the referencing cycle to a strong reference on the Reactive and Rx extensions of the library? 馃槃

I can't offer much input here because I haven't used Rx in months.
But I'm a little confused because did we not purposely make this change in #1311?
The discussion is in #1380 and #1294

I've read the discussions, so I think that the issue is how Moya is interpreting the contracts established by Rx and ReactiveSwift, an Observable, a Signal, a SignalProducer a Promise or any other implementation of the Rx contract, where its lifetime should've always be able to be observed. I cannot have an Observable that, after subscribing, neither disposed, next, error nor complete events are received and thus, having a zombie Observable, not dead, not alive.
I'm with the team that the behavior should be consistent between Rx and ReactiveSwift, so retaining the Provider is key to respect those contracts. 馃殌

Yes, we changed it to unify the behavior across all the platforms. Moya's general design from the very beginning was to keep reference to the provider as long as you need it. We never really supported the behavior when you released the provider before you got the response. Thus we fixed some of the methods from reactive libraries as well.

Personally I'm against the change to retain only for reactive extensions, as we should be consistent with provider's behavior. Maybe we could send an error/failure event, but that's all I would do.

Alamofire is not releasing anything as it's using singletons, same for URLSession. We are not using singletons, thus why I also agree with releasing the resources whenever the provider is deallocated.

Also, to have a non-hacky solution to this one, creating your own method that does not release resources whenever request didn't complete is pretty straight-forward.

Yeah, that's why I introduced the .retaining operator above. That retains any object chosen by the developer along the lifetime of the observable. Just expanding the reasoning to that the finishing the observable without any event is not the correct approach, but I understand the reasons of why this is happening, but I really think that this is something that should be fixed, as it is not a correct Rx nor ReactiveSwift behavior. Thank you for the feedback!!

Missclick! 馃槄

Also, to have a non-hacky solution to this one, creating your own method that does not release resources whenever request didn't complete is pretty straight-forward.

@sunshinejr would there be any drawbacks in doing this? From what I understand after reading this thread, this seems to be more of a design concern rather than a retain cycle bug.

Instead of making another self-retaining method in the reactive extension, what's your opinion on making the provider a global variable?

@esam091 most implementations I've seen have a global provider - it's what Eidolon does too

@AndrewSB I was thinking storing them in the global scope, but Eidolon gives me a good idea of storing them in each view controllers, thanks.

Hi all,

I've just read the discussion. I've just started to adopt RxSwift & Moya. I try to learn basic concepts and want to use a Network class. I created static func, and the request all works well with Moya but I have a warning which is "Result of the call to 'subscribe' is unused". What might be the reason for it?

A little digging, I added the disposeBag at the end but if I do this, my request doesn't work.
So I guess I have to use disposeBag to prevent memory leaks but how?

Could you show me the path? :)

screen shot 2018-04-12 at 15 02 48

my request doesn't work

What message do you get in console? @goktugaral

@jdisho
Nothing, just "Status Code: 200, Data Length: 286"

You either skip to retain the return value of the function (i.e., let _ = provider.rx.....) or retain it in a local variable in the scope of the function, but it seems that you won't do anything with that constant, which is a Disposable btw (i.e., let disposable = provider.rx....).
That being said, the Disposable should always be returned in a Rx function to allow the developer to control the lifetime of the Observable.

Was this page helpful?
0 / 5 - 0 ratings