kotman12 commented on PR #11955: URL: https://github.com/apache/lucene/pull/11955#issuecomment-1321155887
Interesting .. this synchronization might not actually be doing anything after all. But I still think there are some thread-unsafe calls here regarding the integration as a whole which make me a bit paranoid. For instance, there seems to be some code paths that which interact with the singleton factory of the `POSModel`. The `POSModel` itself is a singleton that is stored in this static `posTaggerModels` map. If you follow ``` OpenNLPPOSFilterFactory.create -> NLPPOSTaggerOp.getPOSTagger -> NLPPOSTaggerOp::new -> POSTaggerME::new -> POSTaggerFactory.getPOSContextGenerator -> ConfigurablePOSContextGenerator::new -> POSTaggerFactory.createFeatureGenerators ``` you will see it end up here https://github.com/apache/opennlp/blob/6c9e604e74c941935a3c11f9ba91a7c0f8e1ec4c/opennlp-tools/src/main/java/opennlp/tools/postag/POSTaggerFactory.java#L149 which at first glance appears like a thread-unsafe lazy initialization. I also remember there being other examples. Having said all that, I can't immediately say whether or not the methods you have unsynchronized actually needed to be that way and am scratching my head why they were synched in the first place. But some of these kinds of issues are tricky to pin down even with tests. -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
