Reentrancy issue

[fpillet]

If a feedback observable immediately triggers an event, it won't be recursively called with the new state unless the scheduler is asynchronous.

In the attached example project, I'm using MainScheduler.instance as the selected scheduler. Switching to MainScheduler.asyncInstance unlocks the proper behavior.

While this is not specifically an issue, the fact that there is no runtime warning about reentrancy can make the issue difficult to figure out by inexperienced users.

RxFeedbackSyncIssue.zip

[kzaher]

Hi @fpillet ,

I'm planning to do some further improvements to fix reentrancy issues and add some additional guarantees, they should also solve this issue :)

I've underestimated the amount of time needed to figure all of these things out :) This looks like a simple concept on surface but the devil is in the details :)

[fpillet]

It somewhat looks like differentiating Scheduler and AsyncScheduler protocols would help enforce these guarantees for that kind of API. Would require a change up in RxSwift.

[kzaher]

I would like to make this to work without any assumptions about how schedulers behave. I've found solutions that could fix this issue without making any assumptions, but I would also like to solve some other stale data issues that could theoretically happen :)

I'm now trying to evaluate different solutions for those two issues and how do they play along :)
I can solve that other "staleness" issue if I assume that:

• scheduler is serial
• I change the interface a bit so I can pass scheduler to feedback loops and rearrange react methods a bit.

... but that doesn't prevent someone from designing their own feedback loops in a non optimal way.

For example:

Observable.system(
				initialState: State.first,
				reduce: { (state: State, event: Event) -> State in
					switch event {
						case .toFirst:
							return .first
						case .toSecond:
							return .second
					}
				},
				scheduler: MainScheduler.instance,
				feedback: [
                    { $0.map { _ in .toSecond } },
                    { $0.map { _ in .toFirst } }
                ] as [(Observable<State>) -> Observable<Event>]
).subscribe()

If you run this code the memory will monotonically increase. That indicates that there is a lot of events corresponding to stale state being enqueued on main dispatch queue. One state change causes two events to be queued immediately.

There is quite a lot of different edge cases and I'll need to probably give up solving all of them :), but at least I can solve this one :)

[ixrevo]

The issue with a synchronous scheduler and immediate events from feedbacks almost prevented me from using RxFeedback in my production app. Fortunately I figured it out that I should use async scheduler.

@kzaher could you describe how you are planning to fix this issue? In a couple of words, at least. Or it's too complicated to put in words? :)

[kzaher]

Hi guys,

I've pushed my latest changes to master branch and they should solve these issues. Could you please check them out?

[ixrevo]

Hi @kzaher ,
0.3.1 fixed almost all my issues with a reentrancy anomaly, except one, where one feedback triggers side-effect which triggers other event before the event from the 1st feedback was emitted.

--Feedback-----------------------Event1----
--------------SideEffect--Event2-------------

I can try to create a sample project with example of this issue if you are interested.

And thanks for your update!

[kzaher]

@ixrevo Did you try running release version or did you run debug version.

Debug version has some reentrance guards turned off.
https://github.com/kzaher/RxFeedback/blob/master/Sources/RxFeedback/ObservableType%2BRxFeedback.swift#L70

I will figure out how to create a warning in DEBUG mode with those safety features turned on.

[ixrevo]

@kzaher I'm running debug version.

[kzaher]

@ixrevo Does release version work ok and doesn't have the issue you are referring to?

[ixrevo]

@kzaher After some testing there are results from my project and @fpillet 's sample app:

• a debug build triggers the reentrancy warning in Rx.swfit, but functionality is still working.
• a release build doesn't trigger the warning and functionality is working just fine.

All as expected, I suppose.

By the way, I'm sorry that I'm asking for more of your precious time, but I would like to ask the right way to silence the warning.

I've came up with this one:

var connectivityStatusLoop: (ObservableSchedulerContext<AppState>) -> Observable<AppEvent> {
        return { observableSchedulerContext in
            self._watchSessionManager
                .connectivityStatus
                .map { .connectivityStatusChanged($0) }
                .observeOn(observableSchedulerContext.scheduler)
        }
    }

Am I doing something wrong? Is there some right or better solution?

And if you have some work (not some hardcore stuff, because I'm still learning) in your Rx repositories that you can delegate to me, I am really happy to contribute and open a PR.

[kzaher]

@ixrevo Maybe you aren't using serial scheduler hard to tell. I'll assume this issue is fixed then. Feel free to reopen it if it still isn't resolved.