Python debugging support #3075
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package io.deephaven.util.thread; | ||
|
|
||
| import io.deephaven.configuration.Configuration; | ||
|
|
||
| import java.lang.reflect.InvocationTargetException; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| /** | ||
| * Extension point to allow threads that will run user code from within the platform to be controlled by configuration. | ||
| */ | ||
| public interface ThreadInitializationFactory { | ||
| /* private */ String[] CONFIGURED_INITIALIZATION_TYPES = | ||
| Configuration.getInstance().getStringArrayFromProperty("thread.initialization"); | ||
| /* private */ List<ThreadInitializationFactory> INITIALIZERS = Arrays.stream(CONFIGURED_INITIALIZATION_TYPES) | ||
|
There was a problem hiding this comment. I'm wondering, do we think there are occasions for more than 1 ThreadInitializationFactory? There was a problem hiding this comment. I have assumed for now that each factory should be created once, but it if wants per-thread state, it should do so by putting that state in the For example, the current |
||
| .filter(str -> !str.isBlank()) | ||
| .map(type -> { | ||
| try { | ||
| // noinspection unchecked | ||
| Class<? extends ThreadInitializationFactory> clazz = | ||
| (Class<? extends ThreadInitializationFactory>) Class.forName(type); | ||
| return clazz.getDeclaredConstructor().newInstance(); | ||
| } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | ||
| | InstantiationException | IllegalAccessException e) { | ||
| throw new IllegalArgumentException( | ||
| "Error instantiating initializer " + type + ", please check configuration"); | ||
| } | ||
| }) | ||
| .collect(Collectors.toUnmodifiableList()); | ||
|
|
||
| /** | ||
| * Chains configured initializers to run before/around any given runnable, returning a runnable intended to be run | ||
| * by a new thread. | ||
| */ | ||
| static Runnable wrapRunnable(Runnable runnable) { | ||
| Runnable acc = runnable; | ||
| for (ThreadInitializationFactory INITIALIZER : INITIALIZERS) { | ||
| acc = INITIALIZER.createInitializer(acc); | ||
| } | ||
| return acc; | ||
| } | ||
|
Comment on lines
+36
to
+42
There was a problem hiding this comment. Just to be sure I understand, the reason for this pattern is to allow for initializer-specific cleanup on exit? Otherwise, a pattern of iterative runnable invocations would seem less error-prone. I suppose either version allows arbitrary hijacking of the intended run method; the current pattern could skip calling the wrapped runnable, but the iterative approach could just never return or throw an exception on termination. There was a problem hiding this comment. It isn't that we need to do cleanup on the way out per se, but that we want to insert a stack frame in python. If we end up dropping this frame (which has some weird side effects when you "step out" of the apparent top-level frame), then we could just run one after the other, no wrapping. The current impl has its option of how it wants to do it - I played with an API that would offer a DSL for either wrapping or prefixing, but didn't really love it, I can try bringing that back if you'd prefer. There was a problem hiding this comment. Okay, in light of the arm64 issues, a wrapper is necessary. The question then will be how we name that, if JavaThread is clear and simple enough, or if we want to use it to get some more info to the user. |
||
|
|
||
| Runnable createInitializer(Runnable runnable); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work if the property doesn't exist or is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It fails if it does not exist, but succeeds if empty (with an empty array).
Due to how defaults work (or rather, don't work), I was under the impression we wanted to generally avoid using them, so dh-defaults.prop now contains this property.