Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Fix infinite loop in ResteasyContextListener when injector has parent #13

Merged
merged 2 commits into from
Feb 3, 2017
Merged

Fix infinite loop in ResteasyContextListener when injector has parent #13

merged 2 commits into from
Feb 3, 2017

Conversation

jpeg
Copy link
Contributor

@jpeg jpeg commented Jan 28, 2017

What was changed? Why is this necessary?

An infinite loop would occur in ResteasyContextListener if the Guice injector had a parent injector.

How was it tested?

Added new unit test and verified that it resulted in an infinite loop before the fix was implemented.

How to test

This is bare minimum acceptable testing

  • mvn clean install -U

Reviewers

* Processes module bindings in the Guice injector.
*/
@VisibleForTesting
public void processInjector(ServletContext context, Injector injector) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somehow I am not comfortable having this method. Since this is the initial point of a Web application can the Injector that is created in this class even have a parent Injector ? What if we remove this loop completely and just have the processor process the injector that is created in this class ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other way to detect a loop is to use run a fast-slow runner technique to detect the loop in a LinkedList. Seems like a overkill but may be worth trying it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced the infinite loop could happen at all or that the while loop is even needed at all. Since the context listener creates the injector is there even any possible way for the injector to have a parent injector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the while loop: b666e0a

@nimeshsubramanian
Copy link

issue #12

@sparuvu sparuvu merged commit 0592957 into cerner:master Feb 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants