Add RecursionSafeAsyncCaller: safe to use for recursive calls #3
Conversation
| } else { | ||
| // Case B: Defer to a separate thread | ||
| // This happens when recursion depth of the current thread is larger than limit, to | ||
| // avoid stack overflow. |
There was a problem hiding this comment.
optional: prefer block comments for multiline comments.
| @Override | ||
| public void execute(final Runnable runnable) { | ||
| // Use thread local counter for recursionDepth | ||
| Integer recursionDepth = recursionDepthPerThread.get(); |
There was a problem hiding this comment.
make final. You can avoid incrementing the local variable with:
final Integer recursionDepth = recursionDepthPerThread.get();
recursionDepthPerThread.set(recursionDepth + 1);
/* */
recursionDepthPerThread.set(recursionDepth);There was a problem hiding this comment.
Have fixed
|
|
||
| private StackTraceElement[] stack = new StackTraceElement[0]; | ||
|
|
||
| @SuppressWarnings("unchecked") |
There was a problem hiding this comment.
You can avoid this with:
@RunWith(MockitoJUnitRunner.class)
public class RecursionSafeAsyncCallerTest {
@Mock
public FutureDone<Object> done;
/* */
}
There was a problem hiding this comment.
Have fixed
|
|
||
| @Override | ||
| public <T> void resolve(final FutureDone<T> handle, final T result) { | ||
| execute(new Runnable() { |
There was a problem hiding this comment.
Target version is 1.8, prefer lambdas:
execute(() -> caller.resolve(handle, result));There was a problem hiding this comment.
Thanks, I've fixed
|
Need a way to activate the stack guard, my suggestion would be to add a an option to Also remember to squash your commits. After that, this looks good to me. |
|
Thank you @udoprog. A couple of thoughts below. Activation: After considering this a bit, I'm proposing that both the current caller and threadedCaller each gets an optional recursion guard instance in front of them. In my opinion it's valuable for both. The normal operation of both callers will remain in the default case, until heavy recursion is detected at which time the stack will be protected by deferring a recursive call. What do you think? Squashing commits: I've done this now. Looks good? |
|
Commited a suggestion for how to activate the stack guard. |
gabrielgerhardsson
force-pushed
the
master
branch
from
11e6fb3 to
931bd54
Compare
gabrielgerhardsson
force-pushed
the
master
branch
from
6a852ec to
d9c6df8
Compare
RecursionSafeAsyncCaller will try to run the call in the current thread as much as possible, while keeping track of recursion to avoid StackOverflowError. If recursion becomes too deep, the next call is deferred to a separate thread (ExecutorService thread pool). State is kept per-thread - will be tracked for any thread that passes this code. It is important to choose a suitable maximum recursion depth.
Add option to TinyAsyncBuilder to make use of RecursionSafeAsyncCaller in front of both the non-threaded caller and threadedCaller
gabrielgerhardsson
force-pushed
the
master
branch
from
d9c6df8 to
0cbff23
Compare
3 participants
RecursionSafeAsyncCaller will try to run the call in the current thread as much as possible, while keeping track of recursion to avoid StackOverflowException in the thread. If recursion becomes too deep, the next call is deferred to a separate thread (normal thread pool). State is kept per-thread - stack overflow will be avoided for any thread that passes this code. It is vital to choose a suitable maximum recursion depth.
I should have split out the comment commit to a separate pull request I guess. Will do that next time.