Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition during annotation parsing #1819

Closed
vootelerotov opened this issue Sep 13, 2017 · 2 comments
Closed

Race condition during annotation parsing #1819

vootelerotov opened this issue Sep 13, 2017 · 2 comments
Assignees

Comments

@vootelerotov
Copy link

vootelerotov commented Sep 13, 2017

In org.eclipse.jetty.annotations.AnnotationParser the method isParsed is used in multiple places to guard against duplicate parsing of a class. The method relies on class name to be present in set _parsedClassedNames. The class name is added to _parsedClassedNames in AnnotationParser$MyClassVisitor#visit.

When thread A has started parsing a class but has yet to hit the visit method, thread B could start parsing the same class, passing the isParsed check. This could cause a class to be parsed twice. As annotations are parsed in parallel, this can happen in practice.

A more concrete example would be an implementation of ServletContextListener, annotated with @WebListener, being present in multiple JAR files in WEB-INF/lib. If the annotation is picked up from 2 locations during the scan then the listener is run twice.

I have attached a simple app (repro-servlet-context-listener.zip) to this ticket that I managed to reproduce the issue with, although not reliably. Adding a thread sleep in front of the code flagging the class as parsed should help with the reliability and so did a breakpoint evaluating Thread.sleep(1000).

I do understand that having multiple entries of a class on the classpath is a flawed setup but it would be great if the issues caused by it would show up in a more deterministic way.

Let me know if there is anything I have missed.

@janbartel janbartel self-assigned this Sep 13, 2017
@janbartel
Copy link
Contributor

@vootelerotov even if we scanned the same classname only once, you still wouldn't necessarily have a deterministic outcome, because we can't guarantee the order in which the jars would be examined and thus which classfile would be seen - unless of course the jars are named fragments and you're using a <relative> or <absolute> ordering to control the ordering. In which case you could use your ordering to exclude the jar with the duplicate class :)

That said, I'll take a look and see what we can do to improve the situation and emit some warnings perhaps.

janbartel added a commit that referenced this issue Sep 14, 2017
@janbartel
Copy link
Contributor

I've changed the AnnotationParser so that it makes no attempt to only scan the same class once: if the same class is present in duplicate jars or also in WEB-INF/classes then it will be scanned twice. Jetty will now put out a warning of the duplicates, giving the locations of each.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants