krickert commented on code in PR #1003:
URL: https://github.com/apache/opennlp/pull/1003#discussion_r3059041213
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java:
##########
@@ -21,62 +21,90 @@
import java.util.ArrayList;
import java.util.List;
+import opennlp.tools.commons.ThreadSafe;
import opennlp.tools.util.Cache;
/**
* Caches features of the aggregated {@link AdaptiveFeatureGenerator
generators}.
+ * <p>
+ * The cache is maintained per-thread via {@link ThreadLocal}, making this
class safe for
+ * concurrent use from multiple threads. Each thread gets its own independent
cache that is
+ * cleared when a new sentence (token array) is encountered.
+ * <p>
+ * <b>Note:</b> In container environments with classloader isolation (e.g.
Jakarta EE),
+ * {@link ThreadLocal} state may pin the classloader. Ensure instances do not
outlive
+ * the application's lifecycle, or call {@link ThreadLocal#remove()} on pooled
threads.
*
* @see Cache
*/
+@ThreadSafe
public class CachedFeatureGenerator implements AdaptiveFeatureGenerator {
Review Comment:
I left this class without `AutoCloseable` for now. Unlike `BeamSearch`,
there is no single `ThreadLocal` handle to clear from `close()` without
exposing internals, and the Javadoc already points people at
`ThreadLocal#remove()` for pooled threads.
If we want symmetry with the ME wrappers I am open to a follow-up, but I did
not want to bolt on a half-hearted `close()` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]