Dynamic Thread Predicate doesn't work on thread that installs BlockHound about blockhound

[wburns]

Looks good to me @bsideup, thanks!

[wburns]

Luckily the workaround is simple enough (albeit ugly), I can just wait for 1.0.4.RELEASE.

Thanks again!

[bsideup]

Hi @wburns,

Thanks for reporting, I will look into it.

Meanwhile, could you please clarify why you need this?

When I set all threads to be dynamic

[wburns]

Meanwhile, could you please clarify why you need this?

When I set all threads to be dynamic

That just simplifies the example. However the core problem is if the thread which installs BlockHound runs the test then it runs into this issue, from say IDEA.

[bsideup]

I understand that this is a simplification, but I wanted to know what are you trying to achieve, perhaps there is an easier way.

[wburns]

The core of what I want to do is I am running testng and I may invoke some code that should be non blocking. However because this is invoked in the testng thread it can block for other reasons. I don't want to have to define all the exclusions for a test framework to test such things.

Now one way to fix this is to invoke the code I want to not block solely in a separate thread, but this makes the tests quite hard to write and maintain. And not to mention can run into other bugs as I have to change threads for an operation that wouldn't normally.

I saw you added dynamic test predicates in 1.0.3 and thought I could utilize that so I could mark specific blocks of code as it shouldn't block during it or even replace components with delegating wrappers to ensure the underlying method call doesn't block. But the calling code itself can still block as necessary.

For example this can be a way to define a given code, invoked in the same thread would be checked for non blocking

   public static <V> V ensureNonBlocking(Supplier<V> supplier) {
      isNonBlocking.set(Boolean.TRUE);
      try {
         return supplier.get();
      } finally {
         isNonBlocking.set(Boolean.FALSE);
      }
   }

The isNonBlocking variable is a ThreadLocal which is checked by the non blocking thread predicate when the thread is dynamic.

I guess if you had some sort of static method in BlockHound to say ensure this code is non blocking similar to above, would work for me as well.

[wburns]

So with a method like that I can write a delegate wrapper that allows me to make sure whenever this object is invoked that it doesn't block.

public abstract class EnsureNonBlockingStore<K, V> extends DelegatingNonBlockingStore<K, V> implements NonBlockingStore<K, V> {

   protected abstract NonBlockingStore<K, V> delegate();
   
   @Override
   public CompletionStage<Void> start(InitializationContext ctx) {
      return TestHelper.ensureNonBlocking(() -> delegate().start(ctx));
   }

   @Override
   public CompletionStage<Void> stop() {
      return TestHelper.ensureNonBlocking(() -> delegate().stop());
   }

  ... ommitted rest of methods ...
}

The test code however could still be blocking, such as store.stop().toCompletableFuture().join().

[bsideup]

@wburns sorry, pressed wrong button 😅

Now one way to fix this is to invoke the code I want to not block solely in a separate thread, but this makes the tests quite hard to write and maintain

Since you can do it in TestHelper.ensureNonBlocking, is it really that hard? That's the way I'd recommend.

That said, I kinda like the idea of dynamically changing the current thread's blocking status in tests, will try to fix it.

[wburns]

Since you can do it in TestHelper.ensureNonBlocking, is it really that hard? That's the way I'd recommend.

Oh I am perfectly fine doing this. That isn't the problem. The problem is the dynamic predicate is evaluated with the default dynamic predicate which is false on the invoking thread because it loads a Class. The invoking thread can therefore never have dynamic predicates. Which is why I have to use the ugly workaround of spawning a thread for sole purpose of installing BlockHound.

I created #123 which should show how it could be resolved.

[bsideup]

@wburns I just submitted #124 that should solve it, please have a look

[bsideup]

@wburns thanks for having a look 💯

Once merged, it will be available as 1.0.4.BUILD-SNAPSHOT.
Alternatively, you can use https://jitpack.io to grab it directly from the PR :)

[bsideup]

@wburns released 👍

[wburns]

Thanks!