Issue 635 in google-guice: GuiceFilter uses wrong instance of FilterPipeline if used with multiple servlet context and multiple injectors

[google...@googlecode.com]

Status: New
Owner: ----

New issue 635 by carsten....@gmail.com: GuiceFilter uses wrong instance of
FilterPipeline if used with multiple servlet context and multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

In short:

Servlet Container: Jetty 8.0.0 M3
Guice: 3.0 + Servlet Extension (3.0)

If i create multiple servlet context and configure each of them with a
dedicated injector based on a dedicated servlet module, GuiceFilter aquires
always the FilterPipeline according to the last added servlet
context/injector - no matter which servlet context my HTTP request leads to.

e.g.

Servlet Context A (context path: /a)
Configured with dedicated GuiceFilter & GuiceServletContextListener
providing Injector holding
Servlet Module incl. serve("/hello", HelloAServlet.class)

Servlet Context B (context path: /b)
Configured with dedicated GuiceFilter & GuiceServletContextListener
providing Injector holding
Servlet Module incl. serve("/hello", HelloBServlet.class)

If i add mentioned context in listed order, i will always receive
HelloBServlet.class, no matter if i invoke /hello/a - or /hello/b in my
browser.

I did not spend much time yet to find a clean solution, but what i can say
is that some debugging in GuiceFilter tells me that it somehow uses the
wrong (Managed)FilterPipeline on request / GuiceFilter.doFilter - but it is
associated to the correct servlet context.

So if (i know, not clean solution) i extend GuiceFilter.setPipeline to put
the here given FilterPipeline (which is still correct at this time) into
provided servlet context AND then use this pipeline again in
GuiceFilter.doFilter - all problems are solved.

[google...@googlecode.com]

Comment #1 on issue 635 by carsten....@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

"If i add mentioned context in listed order, i will always receive

HelloBServlet.class, no matter if i invoke /hello/a - or /hello/b in my
browser."

Of course i mean:

If i add mentioned context in listed order, i will always receive

HelloBServlet.class, no matter if i invoke /a/hello - or /b/hello in my
browser.

[google...@googlecode.com]

Comment #2 on issue 635 by carsten....@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Of course i mean:

[google...@googlecode.com]

Comment #3 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

I just ran into the exact same problem. See issue 637. I think I've got an
idea on how to fix this.

[google...@googlecode.com]

Comment #4 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

I've attached a patch that fixes the problem (while maintaining backwards
compatibility).

Attachments:
guice-filter.patch 4.6 KB

[google...@googlecode.com]

Comment #5 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Please note that I am still experiencing race conditions with this patch,
but far less frequently. Sometimes I see GuiceFilter.init() invoked without
GuiceServletContextFilter and this leads to the original pipeline collision
that was reported.

I suspect the problem is Grizzly-specific since it is the one instantiating
GuiceFilter and GuiceServletContextFilter and I'm getting some exceptions
with stack-traces that are purely Grizzly-related.

I need someone else to test the patch on their end, to isolate whether the
problem is specific to the patch or specific to Grizzly.

[google...@googlecode.com]

Comment #6 on issue 635 by carsten....@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Sorry, i did not have had the time yet to further take care for this issue.
Thank you for your response / sharing the problem. I will definitly have a
look at your patch and give you feedback during next week.

As i mentioned in my original post, i already have a kind of hotfix which i
am currently using to proceed with development. I did not experience
further problems with this solution yet. Putting the correct FilterPipeline
into correct ServletContext seems to be fine. I could not spend more time
and search the problem's real source and thereby find its real solution,
yet.

Anyway, main reason for this ticket for me is that i hope for some official
feedback. Since my project is business relevant, i do not really want to
move the issue to a later time by patching 3rd party libraries.

I would rather like to get some information about:

- If we can hope for an official update/release that solves this problem -
and if yes, when could this happen?
- If there is a workaround without touching the libraries?
- If it is just my/our fault and what i want is already working fine.

[google...@googlecode.com]

Comment #7 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

FYI, I have a similar (but smaller) patch in issue 618 which basically
exposes the FilterPipeline API so that third-party extensions can implement
their own delegation strategies. We're successfully using this to provide
servlets from multiple injectors:
https://github.com/sonatype/nexus/blob/guice-servlet/nexus/nexus-web-utils/src/main/java/org/sonatype/nexus/web/NexusGuiceFilter.java

[google...@googlecode.com]

Comment #8 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Issue 637 has been merged into this issue.

[google...@googlecode.com]

Comment #9 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

mcculls,

In the case of issue 618, who would invoke setInjectedPipeline()? As far as
I can tell, I have no way of accessing GuiceFilter from
GuiceServletContextListener in order to invoke setInjectedPipeline().

[google...@googlecode.com]

Comment #10 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

It's visible from any subclass of GuiceFilter, so you just need to extend
GuiceFilter with your desired strategy for locating pipelines and use that
subclass in your web.xml (or wherever you configure the filter-class). Note
that the static functionality in the original base class is unchanged, all
the subclass affects is the managed/injected functionality.

Note: the patch in issue 618 may well not be the best solution, it's just
the simplest solution I've found so far.

[google...@googlecode.com]

Comment #11 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

mcculls,

The key point is "with your desired strategy for locating pipelines". In my
case, once the Injector is created by GuiceServletContextListener I'd like
to set the pipeline to injector.getInstance(FilterPipeline.class).
Unfortunately, I don't have access to the GuiceFilter instance associated
with the current server.

My patch moves this code into GuiceFilter.init() simply because that's the
only way I found for linking GuiceFilter and GuiceServletContextListener...
Now that I think about it, I guess I could still implement the same thing
using your patch simply by overriding Filter.init() in my GuiceFilter
subclass.

One final point: I don't understand why GuiceFilter and
GuiceServletContextListener need to be separate entities. If you're going
to require users to subclass GuiceFilter why not simply merge the two?
Asking users to introduce two separate classes into web.xml in order to
take advantage of Guice is accident-prone. We're better off having one
class (especially since the two need to share data).

[google...@googlecode.com]

Comment #12 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

> The key point is "with your desired strategy for locating pipelines". In

> my case, once the Injector is created
> by GuiceServletContextListener I'd like to set the pipeline to
> injector.getInstance(FilterPipeline.class).

Wouldn't you want to use a delegating approach, so you can handle multiple
pipelines at the same time?

> Unfortunately, I don't have access to the GuiceFilter instance associated
> with the current server.

Yes, we have control over both the Filter and ServletContextListener(s) in
our prototype application, so we can therefore tell the Filter about new
injectors. We use static injection to inject a list of pipelines - this
list is then updated as servlet injectors are created. Our servlets share
the same basic config, just like how Guice servlets extend the same base
servlet module, and this is how we "call-home" as each injector is
initialized. Then as requests come in we simply iterate over the pipeline
sequence looking for one that can handle the request.

> My patch moves this code into GuiceFilter.init() simply because that's
> the only way I found for linking
> GuiceFilter and GuiceServletContextListener... Now that I think about it,
> I guess I could still implement
> the same thing using your patch simply by overriding Filter.init() in my
> GuiceFilter subclass.

Exactly, if you extend GuiceFilter (plus the patch in Issue 618) then you
can choose how they're linked - personally I'd use some form of injection
to loosen the coupling between the filter and context listener.

> One final point: I don't understand why GuiceFilter and
> GuiceServletContextListener need to be separate
> entities. If you're going to require users to subclass GuiceFilter why
> not simply merge the two? Asking
> users to introduce two separate classes into web.xml in order to take
> advantage of Guice is accident-prone.
> We're better off having one class (especially since the two need to share
> data).

I assume it's separation of concerns - GuiceFilter is responsible for
filtering and dispatching requests, whereas GuiceServletContextListener is
responsible for setting up the injector(s). This makes sense to me, and in
fact it wouldn't make sense to combine these two classes for our particular
application. Besides, afaict most people shouldn't have to extend
GuiceFilter, and those that do need to extend it should know enough to be
able to manage the two classes.

Anyway, this is just what works for us (I prefer small patches) - it may
not suit your particular application.

[google...@googlecode.com]

Comment #13 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

mcculls,

> Wouldn't you want to use a delegating approach, so you can handle
> multiple pipelines at the same time?

What does a delegating approach really mean in this case? I want to run
multiple unit tests in parallel where each @Test method gets a separate
Injector and web server to run against. A delegating approach sounds like
we're saying: if server 1 cannot handle a request, server 2 should try to,
then server 3 and so on. That sounds more like fail-over to me and doesn't
really make sense for my unit test use-case.

> Besides, afaict most people shouldn't have to extend GuiceFilter, and
> those that do need to extend it should know enough to be able to manage
> the two classes.

Agreed, which is why I'd prefer adding code to GuiceFilter that
automatically tries grabbing a FilterPipeline from
GuiceServletContextListener at init() time. If I understand your use-case
correctly, you want to be able to modify the filter pipeline multiple times
over the lifetime of the Filter (whereas I only want it set once). What we
can do is have init() pick up reasonable defaults and add the setter method
you proposed. This way both our use-cases are possible.

[google...@googlecode.com]

Comment #14 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

> > Wouldn't you want to use a delegating approach, so you can handle

> multiple pipelines at the same time?

> What does a delegating approach really mean in this case? I want to run
> multiple unit tests in
> parallel where each @Test method gets a separate Injector and web server
> to run against.

Usually I'd run that sort of test (ie. more integration test than unit
test) in an isolated classloader or jvm to avoid static clashes elsewhere
in the app or container, but I can see what you're trying to achieve.

This issue covers many use-cases, and while a last-one-wins approach is
fine for your test use-case it won't work for our situation where we want
to manage multiple injectors concurrently under a single filter instance.
For example, injector A maps /a -> servletA while at the same time injector
B maps /b -> servletB.

> A delegating approach sounds like we're saying: if server 1 cannot handle
> a request, server 2
> should try to, then server 3 and so on. That sounds more like fail-over
> to me and doesn't really
> make sense for my unit test use-case.

Our use-case is a plugin system, with one injector per-plugin running
inside the same servlet container. Each plugin can contribute servlet
bindings, so we have a central delegating filter that checks each active
pipeline to see whether it can handle the incoming request.

> > Besides, afaict most people shouldn't have to extend GuiceFilter, and
> those that do need to
> > extend it should know enough to be able to manage the two classes.

> Agreed, which is why I'd prefer adding code to GuiceFilter that
> automatically tries grabbing a
> FilterPipeline from GuiceServletContextListener at init() time. If I
> understand your use-case
> correctly, you want to be able to modify the filter pipeline multiple
> times over the lifetime of
> the Filter (whereas I only want it set once). What we can do is have
> init() pick up reasonable
> defaults and add the setter method you proposed. This way both our
> use-cases are possible.

Sure, you could easily build a single solution on top of the patch from
issue 618.

[google...@googlecode.com]

Comment #15 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

mcculls,

> Usually I'd run that sort of test (ie. more integration test than unit
> test) in an isolated classloader or jvm to avoid static clashes elsewhere
> in the app or container, but I can see what you're trying to achieve.

That's actually a very good idea though I have a feeling I'm going to have
a hard time convincing the Surefire committers to add such a feature. Doing
this myself on a per-@Test basis sounds painful.

I'm fine with using your patch so long as:

1. Currently MULTIPLE_INJECTORS_WARNING is logged if the static pipeline
value is clobbered. I propose logging this warning only if the clobbered
value actually gets used (this is included in my patch). Otherwise we're
logging warnings that aren't actually relevant.

2. We document how to run multiple instances of GuiceFilter per JVM (my
use-case). If subclassing is required, so be it, so long as we post the
relevant code-sniplet somewhere.

[google...@googlecode.com]

Comment #16 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Filed http://jira.codehaus.org/browse/SUREFIRE-749

[google...@googlecode.com]

Comment #17 on issue 635 by carsten....@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

I would be fine with this solution too :)

But just to make this clear - as far as i understand, the current guice
release is supposed to support multiple injectors?

So should this include above mentioned use case and reason for this ticket
- and will there be a fix/change? Or is discussed solution which bypasses
the problem by using extended visibility of implementation details the way
to go?

In either case, i also think it would be better to have more specific
documentation on this ...

[google...@googlecode.com]

Comment #18 on issue 635 by carsten....@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Just to make this clear - as far as i understand, the current guice release

is supposed to support multiple injectors?

So should this include above mentioned use case and reason for this ticket

- and will there be a fix/change? Or is discussed solution which by-passes

the problem by using extended visibility of implementation details the way
to go?

Wouldn't this be a massive break into the 'guice api'? Using this patch &
extending visibility means i rely on implementation details that might
change in future.. Besides other possible problems (e.g. in concurrent
context) i am not able to foresee now.

Anyway, i also think it would be better to have more specific documentation
on how to implement required use case, if there wont be an official
way/fix/change ...

[google...@googlecode.com]

Comment #19 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Having given this some more thought, I believe using different ClassLoaders
is more of a band-aid than a proper solution. One of the main selling
points of Guice was that by migrating away from static factories we'd
improve unit test isolation. This is precisely the problem we are
encountering here.

[google...@googlecode.com]

Comment #20 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

mcculls,

Keeping separation of concerns in mind, can you please explain what
GuiceFilter and GuiceServletContextListener are meant to do, independently
of one another?

[google...@googlecode.com]

Comment #21 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

> Having given this some more thought, I believe using different

> ClassLoaders is more of a band-aid than a
> proper solution. One of the main selling points of Guice was that by
> migrating away from static factories
> we'd improve unit test isolation. This is precisely the problem we are
> encountering here.

I didn't say Guice shouldn't support this use-case (it's entirely possible
with either patch) - I was making the point that your tests could always
run into issues with other third-party libraries that rely on static
members, because those tests are spinning up live servlet containers, etc.
and not AFAIK performing any mocking.

So while Guice can certainly help you move away from static factories, it
can't magically replace them in existing code - which is where mocking can
help, because you then don't rely on having a live container.

Anyway, back to the original issue - it would be good to get Dhanji's view
on this...

[google...@googlecode.com]

Comment #22 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

> Keeping separation of concerns in mind, can you please explain what

> GuiceFilter
> and GuiceServletContextListener are meant to do, independently of one
> another?

I believe I answered this in an earlier comment (note: its just my personal
view, I didn't design these classes)

* GuiceFilter is responsible for filtering requests/responses by using the
pipeline(s) injected into it
* GuiceServletContextListener is responsible for creating injectors
on-demand that inject pipelines

So while there is a link between the two - one creates injectors, the other
expects pipeline(s) to be injected - they could conceivably work
independently, in that you could put in your own Filter implementation that
just used the injector created and stashed in the context by the listener.
Or you could write your own listener that used a different pipeline
implementation (assuming the FilterPipeline type was opened up as in my
patch).

Conversely, why should these classes be combined? They implement different
interfaces that have no relation or reference to each other in the servlet
spec, so combining them is imho purely an implementation detail to avoid a
little bit of indirection.

[google...@googlecode.com]

Comment #23 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

mcculls,

I think GuiceFilter has two separate roles:

1. Provide an Injector to registered servlets/filters. Meaning, the servlet
API instantiates GuiceFilter and it injects any registered
servlets/filters. The emphasis here is that without GuiceFilter we wouldn't
have access to an Injector.

2. Replace web.xml. Convert your configuration into type-safe code.

Assuming I understand your use-case correctly, I argue that GuiceFilter
doesn't need to serve different pipelines (more on this below).

Keeping #1 in mind, I'd advocate merging GuiceFilter and
GuiceServletContextListener (such that each GuiceFilter instance would be
bound to exactly one Injector). Users who wish to specify multiple
injectors could simply specify multiple GuiceFilter instances (one per
Injector). The Servlet API's existing filter-chain mechanism would traverse
one GuiceFilter/injector after another.

The merged class would still enable you to move your configuration from
web.xml into code. Instead of having one entry per
GuiceServletContextListener subclass in web.xml, you'd have one line per
GuiceFilter with its own Injector.

Would you be able to implement your use-case in terms of multiple
GuiceFilter subclasses, as mentioned above?

[google...@googlecode.com]

Comment #24 on issue 635 by dizi...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Yabancı dizi izle http://www.dizipop.com/

[google...@googlecode.com]

Comment #25 on issue 635 by DavidB...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

This issue really needs to be fixed. As comment 5 states, with the October
2011 changes, making FilterPipeline public and the
GuiceFilter(FilterPipeline) constructor visible (ideally public, but
protected is workable as well) is sufficient.

The JVM-scope staticness (actually, classloader-scope staticness) of
GuiceFilter is a real blight on the library. The code is in place to not
rely on it accept for backwards compatibility (i.e., dynamic
filterpipelines are supported; the constructor is just not visible). Please
make it visible.

[google...@googlecode.com]

Comment #26 on issue 635 by DavidB...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

This issue really needs to be fixed. As comment 5 on issue 618 states, with

[google...@googlecode.com]

Comment #27 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

It occurs to me that I never stated my use-case, so here it is:

I am attempting to run unit tests concurrently. Each @Test runs against its
own web server. Each web server instantiates its own Injector and (as a
consequence) its own GuiceFilter instance. The problem is that all
instances share the GuiceFilter's static fields.

The goal is two-fold:

1. Allow GuiceFilter to be used in a thread-safe manner.
2. Remove spurious thread-safety warnings (the one about multiple
injectors).

Reducing the logging level is not the same as removing spurious warnings.
If there is no danger, please don't log at all!

How you solve this problem is entirely up to you, but please fix this soon.
Let me know if there is anything I can do to help.

[google...@googlecode.com]

Comment #28 on issue 635 by mccu...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Could you perhaps provide a JUnit based testcase? (See the current tests
under extensions/servlet.)

That would a) help verify any potential solution and b) provide others with
a multi-context example.

[google...@googlecode.com]

Comment #29 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

I'm not sure how to write a JUnit test that verifies the two goals I
mentioned. I don't think unit tests are the right way to validate the
thread-safety of a class. Nor am I sure how to check whether a particular
warning message was logged in the course of an HTTP request.

Why are you asking for a JUnit testcase? Is there any ambiguity in the
stated goals or in how to reproduce the spurious warnings?

[google...@googlecode.com]

Comment #30 on issue 635 by sa...@google.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Tests (junit-based or not) help to ensure the changes fix exactly what is
requested, and that future maintenance on the code doesn't cause
regressions. It's standard practice to want a test confirming every change.

[google...@googlecode.com]

Comment #31 on issue 635 by vaclavsl...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Are there any updates on this issue? This makes practically impossible to
use Guice for multiple Contexts without having to modify it. It even
prompted me to have a look on released versions of Guice and it does almost
seem that Guice is a dead project. This isuues is quite critical for some
use cases and not fixed for over a year :(

[google...@googlecode.com]

Comment #32 on issue 635 by ohads...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Is there any chance it will be fixed?

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[google...@googlecode.com]

Comment #33 on issue 635 by dha...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Are people still using multiple contexts? Doesn't classloader isolation in
say, Tomcat, solve this?

[google...@googlecode.com]

Comment #34 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

The problem isn't multiple contexts. The problem is unit tests, where you
might have multiple servers, each instantiating their own GuiceFilter, but
they are all running inside the same Classloader.

I tried asking both JUnit and TestNG authors to add Classloader isolation
and they both refused claiming this would degrade performance and cause
annoying casting problems.

So, here we are.

[google...@googlecode.com]

Comment #35 on issue 635 by SirR...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

I have a valid use case, where this issue proved to be troublesome. I
applied the current patch presented here and it worked.
Having applications A and B, both packed as EAR having EJB's. Both have
Guice DI, provided by GWTP.

Both applications have it's own instances of the GuiceFilter.
I tried adding A's EJBs classes to the /lib folder of the the B
application, but invoking the EJB proved to be unsuccessful, because of the
classloading issues. FooEJB from A project wasn't the same as the FooEJB at
the B project /lib folder.

I removed the A application from the /lib folder of the B project and
deployed as a dependent module (on JBoss 7.20). Now the B project shares
the EJB classes from A project, but also the GuiceContext. Issues when I
tried load a A application URL and B GuiceFilter would "intercept" and try
to process it. EJB worked fine.
Refactoring A application common classes would work, but that would be too
much work (big blob of legacy code), patching was simpler. Patched worked
on Guice 4.

[google...@googlecode.com]

Comment #36 on issue 635 by dha...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

@cowwoc I see, that's a good point re: tests. The static in GuiceFilter is
quite annoying I agree. However, I want to solve this without breaking the
ease of use of in servlet environments where web.xml instantiates filters
directly.

Let me think about this issue a bit. Im loathe to expose internals as issue
#618 suggests unless absolutely necessary.

[google...@googlecode.com]

Comment #37 on issue 635 by cow...@bbs.darktech.org: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

Remind me again, why do we need a static variable for web.xml instantiated
filters? I've been months/years since I looked at this code, so I forget
the details.

[google...@googlecode.com]

Comment #38 on issue 635 by dha...@gmail.com: GuiceFilter uses wrong

instance of FilterPipeline if used with multiple servlet context and
multiple injectors
http://code.google.com/p/google-guice/issues/detail?id=635

That's legacy--I'm trying to see if there's a way to extricate ourselves
from that mess, but a lot of clients depend on the broken behavior so it's
not trivial.