Should we protect against reentrancy?

[zenparsing]

@jhusain @blesh

Should the wrapped observer protect against reentrancy? Currently it doesn't. In order to protect against reentrancy, we would need to maintain a state flag in the wrapped observer, and if the observer is currently running, then do one of the following:

• Queue the event to be sent later.
• Throw an error immediately (like generators do).

[benlesh]

I believe we added logic for this to Subjects in RxJS 5, but not for Observers themselves. I don't think it's needed, but it doesn't hurt to look into it. /cc @trxcllnt

I guess a user could do something weird like:

let _observer;
new Observable(observer => {
  _observer = observer;
  observer.next('go!');
}).subscribe({
  next(x) {  _observer.next('wee'); }
});

I'm not sure why anyone would try to do that.

[benlesh]

... well... actually hold on.

There's the expand operator in RxJS that actually does this (sorta) by design. Values are both emitted and pumped back into the selector function by sending them back into the subscriber's (observer's) own next.

[benlesh]

... I suppose that's more of an implementation detail of the observer for that operator, and it wouldn't be too hard to develop around a reentrancy check.

[trxcllnt]

@blesh we protect against re-entrant error and complete calls, but not next, because doing it without throwing errors or dropping values necessitates introducing unbounded buffers.

[benlesh]

That's right, thank you for reminding me, @trxcllnt.

[benjamingr]

You can protect against immediate reentrency like @blesh 's example but not chains - that sounds like a reasonable compromise - no?

Also - what's so bad about reentrency in the first place?

[benjamingr]

Can we agree to not protect against reentrency?

[benlesh]

I don't think we should protect against next reentrancy... but, error and complete we probably should, since those are terminal events.

[benjamingr]

@blesh why? I'm wondering if we can/should just throw when trying to next an observable which is completed by default (and let subclasses take care of allowing it in odd cases).

[niklas-wortmann]

I think this could lead to several problems (more from an api users point of view and might be very subjective):

• I wouldn't expect that I need to catch a thrown error after a terminal event
• complete also implies the end of the stream :D

Additionally I don't see added value but added complexity by additional error handling?

[ivan7237d]

@Blesh we protect against re-entrant error and complete calls, but not next, because doing it without throwing errors or dropping values necessitates introducing unbounded buffers.

I'm really curious to find out more detail on why unbounded buffers are considered a deal breaker in this case.