interruptOnTimeout doesn't work in Hystrix 1.4.0-RC5

[adamdyga]

When using Hystrix 1.4.0-RC5, the commands are not interrupted when the command times out.
The following naive test shows how to reproduce this:

public class InterruptTest
{
    private static final Logger LOG = LoggerFactory.getLogger(InterruptTest.class);

    private class SleepingCommand extends HystrixCommand<Void> {

        public SleepingCommand() {
            super(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("mygroup"))
                    .andCommandPropertiesDefaults(HystrixCommandProperties.Setter()
                            .withExecutionIsolationThreadInterruptOnTimeout(true)
                            .withExecutionIsolationThreadTimeoutInMilliseconds(1000)));
        }

        private volatile boolean hasBeenInterrupted;

        public boolean hasBeenInterrupted()
        {
            return hasBeenInterrupted;
        }

        @Override
        protected Void run() throws Exception
        {
            try {
                Thread.sleep(2000);
            }
            catch (InterruptedException e) {
                LOG.error("Interrupted!", e);
                hasBeenInterrupted = true;
            }

            return null;
        }
    }

    @Test
    public void shouldInterruptOnTimeout() throws InterruptedException
    {
        // given
        SleepingCommand cmd = new SleepingCommand();

        // when
        cmd.observe().subscribe();

        // then
        Thread.sleep(3000);
        Assert.assertTrue(cmd.hasBeenInterrupted());
    }
}

It works fine with Hystrix 1.3.18.

[mattrjacobs]

@aadeon This is a bug, but I think the fix has to be made in conjunction with RxJava. I talked to @benjchristensen this morning, and there is some active work around thread interruption happening in RxJava at the moment.

Right now, RxJava is unilaterally deciding to do thread interruption on an unsubscribe. IIRC, up until 1.02, the choice was to do it, and for 1.0.3+, the choice was not to. The point is that there's currently no way for Hystrix to specify this decision on a per-instance basis.

As @benjchristensen make progress on this interaction, I'll put notes here.

In order to not hold up RC6, I'm now targeting this to RC7

[mattrjacobs]

I now have a tentative fix for this issue that respects the property value on whether or not to interrupt the underlying thread, and am awaiting feedback on it. I think the interaction between Hystrix and Rx could be improved and want to see what options there are before committing to the version in #593

[akarnokd]

Currently, we interrupt a ScheduledAction if the unsubscription was called from a different thread the action is running on. If it was called from the same thread, we cancel the underlying future without interruption. This saves us some time for normally completing actions.

@abersnaze's PR was no-op in RxJava but the associated test change suggests there is a custom Scheduler involved which doesn't track its active tasks so unsubscribing its worker doesn't cancel the scheduled tasks. I'm not familiar with how Hystrix interacts with RxJava.

[mattrjacobs]

Fixed in #647