Bug in OnSubscribeFromIterable unsubscribed child logic

[andres-pipicello]

Hello

The code is missing an additional check for child unsubscription after calling the iterator next method. This is needed when using in conjunction with operators like take, that unsubscribes itself in the onNext method.

Regards

[davidmoten]

Sorry, can't see the issue with take, can you elaborate? If the child has unsubscribed during the onNext then the next loop will check for unsubscription and exit.

The iterator next method could take enough time for unsubscription to have happened in the meantime so we could insert another check for unsubscription before emitting. I'll see what the others think of this as I imagine it would compromise performance a bit for common use cases like using java Collections as sources because each emission would require two volatile reads of unsubscribed.

if (o.isUnsubscribed())
    return;
T val = it.next();
if (o.isUnsubscribed()) 
    return;
o.onNext(val);

Is this what you were meaning?

[andres-pipicello]

The problem arised when using an Iterable which produces iterators with a blocking hasNext.
Imagine the case where the hasNext call would block after retrieving 10 elements (so, in the eleventh call). Chaining a take/limit of 10 would still block, even after emiting 10 elements.

We did a quick patch:

while (it.hasNext() && --numToEmit >= 0) {
                            if (o.isUnsubscribed()) {
                                return;
                            }
                            o.onNext(it.next());
                            if (o.isUnsubscribed()) {
                                return;
                            }

                        }

[zsxwing]

How about checking isUnsubscribed before hasNext, such as:

                        while (!o.isUnsubscribed() && it.hasNext() && --numToEmit >= 0) {
                            o.onNext(it.next());
                        }
                        if (o.isUnsubscribed()) {
                            return;
                        }

[davidmoten]

@pelecomepibe yep that does look something to avoid
@zsxwing LGTM

[akarnokd]

Thanks for reporting. Fix released in 1.0.12.