This is a nice one. Once we have that in place, we can probably/hopefully
also provide a hotkey to gracefully cancel queries in the REPL (w/o having
to exit the console itself).
Cheers,
Daniel
On Fri, Oct 16, 2015 at 1:05 PM, Stephen Mallette <spmalle...@gmail.com>
wrote:
> I'm going to frame this in the context of Gremlin Server but I think this
> issue generally applies to any threaded application development done with
> TinkerPop.
>
> Gremlin Server does a number of things to protect itself from crazy scripts
> (i.e. long run scripts that may have been accidentally or maliciously sent
> to it). The one thing it doesn't do perfectly is properly kill running
> scripts in the midst of a long run traversal. It attempts to stop a
> running script by interrupting the thread that is processing it, but if the
> thread is at a point in the script that doesn't respect Thread.interrupt(),
> it basically just chews up that thread until it reaches a spot that does.
>
> I think Traversal could do a better job respecting interrupts. Put in code
> speak, we should look to get this test to pass for all steps:
>
> @Test
> public void shouldRespectThreadInterruption() throws Exception {
> final AtomicBoolean exceptionThrown = new AtomicBoolean(false);
> final CountDownLatch startedIterating = new CountDownLatch(1000);
> final List<Integer> l = IntStream.range(0,
> 1000000).boxed().collect(Collectors.toList());
>
> final Thread t = new Thread(() -> {
> try {
> __.inject(l).unfold().sideEffect(i ->
> startedIterating.countDown()).iterate();
> fail("Should have respected the thread interrupt");
> } catch (Exception ie) {
> exceptionThrown.set(ie instanceof RuntimeException);
> }
> });
>
> t.start();
>
> startedIterating.await();
> t.interrupt();
>
> t.join();
> assertThat(exceptionThrown.get(), CoreMatchers.is(true));
> }
>
> Two things to note...First, the changes to make this test pass shouldn't be
> "big". I got this test to pass with the addition of this line of code in
> FlatMapStep.processNextStart() on the first line after the start of the
> while(true).
>
> if(Thread.interrupted) throw new RuntimeException();
>
> That single line inserted in the right places should get interrupt working
> properly across Traversal.
>
> Second, note that I'm throwing/asserting RuntimeException. I probably
> should be throwing a InterruptedException but that is a checked exception
> and that would really pollute up our Traversal code. So, my suggestion is
> to create our own "InterruptedRuntimeException" that we can trap separately
> that would potentially wrap an actual InterruptedException.
>
> I'd like to tack this issue in to 3.1.0 if possible - please let me know
> your thoughts if you have any.
>
