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

Circular dependency with take(1) reaches Maximum call stack size #158

Closed
Steelfish opened this issue Dec 10, 2016 · 4 comments · Fixed by #159
Closed

Circular dependency with take(1) reaches Maximum call stack size #158

Steelfish opened this issue Dec 10, 2016 · 4 comments · Fixed by #159

Comments

@Steelfish
Copy link
Contributor

Steelfish commented Dec 10, 2016

The following code is able to reach maximum call stack size and I do not see why.

var value$proxy = xs.create(); 
var content$ = value$proxy.startWith(1);
var value$ = xs.of(1).map(_ => content$.take(1)).flatten();
value$proxy.imitate(value$)
content$.addListener({
    next: (x) => bin.log("content$proxy: ", x),
  complete: () => bin.log("COMPLETE"),
  error: () => bin.log("ERROR")
})
value$proxy.addListener({
  next: (x) => bin.log("Value$proxy: ", x),
  complete: () => bin.log("COMPLETE"),
  error: () => bin.log("ERROR")
})

See also the log of the following webpackbin: http://www.webpackbin.com/4JfBs1HQf
(Warning: It might take a while to load and you probably don't want to save anything at the risk of crashing your browser tab)

Should not the take(1) operator prevent the proxystream from reaching maximum call stack size?

I believe this construction worked in xstream 6.0.0

@midnight-wonderer
Copy link
Contributor

The code confused me at first but not matter how fancy the middle transformation is
the simplify version is just

const a = xs.of(1);
a.imitate(a);

same as

const a = xs.never().startWith(1);
a.imitate(a);

and

const a = xs.never().startWith(1);
const b = xs.of(1).map(() => a.take(1)).flatten();
a.imitate(b);

and

const a = xs.never();
const b = a.startWith(1);
const c = xs.of(1).map(() => b.take(1)).flatten();
a.imitate(c);

This proves how fragile mixing Subject functionality into Observable can be. It is not the code that wrong but our mental model that doesn't keep up.

To me, the only mystery left is why the code does not hang immediately at this line.

value$proxy.imitate(value$)

but continue and allow the listener to be added.
Not so interested in figuring it out.

@midnight-wonderer
Copy link
Contributor

Opps on second though I think it is actually involve a bug.

_n(t: T) {
const u = this.out;
if (u === NO) return;
if (this.taken++ < this.max - 1) {
u._n(t);
} else {
u._n(t);
u._c();
}
}

I am on my mobile phone right now and not actually testing.
It seems to me there is no termination case when _n is call consecutively.
this.taken just keep counting and the code never reach _c.

midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 16, 2016
@midnight-wonderer
Copy link
Contributor

The fix is simple but I am currently figuring out how to test.
Feel free to give an early feedback about the change.

midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 17, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 17, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 17, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 17, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 17, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 17, 2016
@midnight-wonderer
Copy link
Contributor

Sorry for a lot of updates referencing this issue on my repo.
I did something wrong and get caught in rebase hell.

midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 21, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 21, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 22, 2016
midnight-wonderer added a commit to midnight-wonderer/xstream that referenced this issue Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants