Thanks very much for the detailed feedback. I can see your points. Would
you like me to try again or just leave it to you experts?
On 17 Feb 2014 17:34, "akarnokd" <notifications@github.com> wrote:

> In rxjava-core/src/main/java/rx/operators/OperationRetry.java:
>
> >
> > -                        @Override
> > -                        public void onCompleted() {
> > -                            observer.onCompleted();
> > -                        }
> > +        private void subscribeToSource() {
>
> You need to perform the subscription on a scheduler such as currentThread
> to avoid stack overflow.
>
This is an excellent point, I'll remember this one!

>  In addition,
>
>    - it is not advised to call subscribe from a constructor,
>
> ok

>
>    - the call from the constructor is not protected by a lock and there
>    is a race condition between subscribed.set and unsubsctibeFromSource,
>
> no race condition because no one can unsubscribe till the
RetrySubscription is constructed. If the sub was moved to a method called
after construction then would use lock.

>
>    - even if using lock, subscribeToSource may be reentered before the
>    subscribe returns and subscribed might not be set to true at all,
>
>  ah yep, indeed

>
>    - you'd need to use MultipleAssignmentSubscription which does the
>    correct lifecycle management for this operator (the original bug was a
>    per-source shared composite instead of a per-observer composite plus the
>    retry counter).
>
> I'll have a look at that. Was a good exercise for me, sounds like the
original source (pre my version) may just need a slight adjustment in line
with your description of the bug. I'll have a look at it anyway out of
interest. Have you got a fix lined up already? Thanks for your time with my
code!
Dave

>
>
> I
> Reply to this email directly or view it on GitHub<https://github.com/Netflix/RxJava/pull/882/files#r9783144>
> .
>