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]

Reply via email to