dazey3 edited a comment on pull request #65: URL: https://github.com/apache/openjpa/pull/65#issuecomment-663070397
@rmannibucau > The toggle never worked reliably Other than with concurrency, which is the reason we are pursuing a fix in the first place, do you have examples of how this toggle doesnt work reliably? > it is saner than a toggle which blindly excludes classes and can miss enhancements. Do you have examples of missed enhancements by the transformer due to this boolean toggle? As I am aware, recursive calls are not used to do transformation/enhancement. This toggle is absolute in it's exclusion, it excludes all reentrance, however I do not know of an issue that demonstrates that to be an incorrect behavior. Is there an open defect to fix that issue if it exists? Can that issue not be fixed separately? @struberg >The whole recursive situation usually does not occur if the agent is attached dynamically, it does not occur if the PCClassFileTransformer is used programmatically etc ... > It will mostly happen if the transformer is used as javaagent and then only for all the classes used in the PCClassFileTransformer code itself. Thank you for verifying this. In a server environment, we are seeing that no transformation recursion is happening as a product of classloading/transforming a class. The current implementation is written in such a way that ALL recursion would be blocked and I do not know of any outstanding issues with that behavior. > Now both the boolean and the exclude list from @rmannibucau serve the same needs. Both are an exit criteria in the classloader iteration for internal classes. I agree that both implementations serve the same needs. However, I prefer the boolean implementation because it maintains the original behavior of excluding ALL possible recursive calls, on the same Thread. If there is an issue with this implementation, I don't see why that can't be fixed in a separate issue that documents THAT issue. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
