Fix TrampolineScheduler NullPointerException

[benjchristensen]

I tried for about 30 minutes to replicate the NPE reported in #1702 but couldn't.
It makes sense reading the code that an unsubscribe could trigger an NPE though so I'm fixing it as per recommendation of @DylanSale even though I can't replicate.
I confirmed that the items are being put in the queue BEFORE the wip variable is incremented, so that concurrency seems okay.

[akarnokd]

I found it always odd to have a threadlocal queue and a per-scheduler wip counter. I can't test this now, but I think the NPE may happen if two tasks are scheduled from different threads on the same worker which makes wip = 2 but the thread that is consuming the tasks only sees its threadlocal queue with 1 item.

[benjchristensen]

It is indeed very odd, an artifact of early RxJava trying to match how Rx.Net did unsubscribes and scheduling. As of 0.17 when lift was added that all went away but TrampolineScheduler has not caught up.

[benjchristensen]

Just added another commit that removes the use of ThreadLocal.

[akarnokd]

Instead of removing it, I would have made wip threadlocal as well. That way, the TrampolineScheduler worked like a current-thread scheduler for everyone and not sharing a queue and hopping threads.

[benjchristensen]

It doesn't need to act like a ThreadLocal. It had a separate queue per Worker just like all Scheduler.Worker impls.

Ben Christensen
310.782.5511
@benjchristensen

On Oct 10, 2014, at 12:44 AM, David Karnok [email protected] wrote:

Instead of removing it, I would have made wip threadlocal as well. That way, the TrampolineScheduler worked like a current-thread scheduler for everyone and not sharing a queue and hopping threads.

—
Reply to this email directly or view it on GitHub.

[akarnokd]

I thought about it a bit and I was mistaken; the Worker needs to keep the tasks in order so threadlocal wip is not good.