Leak for RequestScoped injectables if async server model is used

[glassfishrobot]

The dispose method of a HK2 factory bound to RequestScoped is never called when the async server model is used. I intend to attach a simple unit test using grizzly but we see the same result using Jetty as runtime.

It seems that the RequestScoped.Instance reference count is not decremented often enough to trigger disposal in the async case as we managed to work around the problem with a custom RequestListener that captures the current Instance for async requests and invokes release on it on the FINISHED event (twice as it is responsible for one reference itself).

Affected Versions

[2.12, 2.13]

[glassfishrobot]

Reported by bodewig

[glassfishrobot]

bodewig said: I can't attach anything here, is this correct?

OK, here is my unit test inline

package org.example.jersey_bug;

import javax.inject.Inject;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.container.AsyncResponse;
import javax.ws.rs.container.Suspended;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.Response;

import org.glassfish.hk2.api.Factory;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
import org.glassfish.jersey.process.internal.RequestScoped;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

public class RequestScopedAndAsyncTest extends JerseyTest {

    public static class Injectable {
        // just a placeholder
    }

    public static class InjectableFactory implements Factory<Injectable> {
        private static final Object LOCK = new Object();
        private static int provided;
        private static int balance;

        @Override
        public Injectable provide() {
            synchronized(LOCK) {
provided++;
balance++;
            }
            return new Injectable();
        }

        @Override
        public void dispose(Injectable i) {
            synchronized(LOCK) {
balance--;
            }
        }

        public static void reset() {
            synchronized(LOCK) {
provided = balance = 0;
            }
        }

        public static void assertProvidedOne() {
            synchronized(LOCK) {
Assert.assertEquals(1, provided);
            }
        }

        public static void assertIsBalanced() {
            synchronized(LOCK) {
Assert.assertEquals(0, balance);
            }
        }
    }

    @Path("test")
    public static class TestResource {

        @Inject
        private Injectable injectable;

        @GET
        @Path("sync")
        public Response sync() {
            return Response.noContent().build();
        }

        @GET
        @Path("async")
        public void async(@Suspended AsyncResponse ar) {
            ar.resume(Response.noContent().build());
        }
    }

    @Before
    public void resetCounters() {
        InjectableFactory.reset();
    }

    @Override
    protected Application configure() {
        return new ResourceConfig(TestResource.class)
            .register(new AbstractBinder() {
    @Override
    protected void configure() {
        bindFactory(InjectableFactory.class)
            .to(Injectable.class)
            .in(RequestScoped.class);
    }
});
    }

    @Test
    public void shouldProvideAndDisposeSync() {
        Assert.assertEquals(204, target("/test/sync").request().get().getStatus());
        InjectableFactory.assertProvidedOne();
        InjectableFactory.assertIsBalanced();
    }

    @Test
    public void shouldProvideAndDisposeAsync() {
        Assert.assertEquals(204, target("/test/async").request().get().getStatus());
        InjectableFactory.assertProvidedOne();
        InjectableFactory.assertIsBalanced();
    }

}

[glassfishrobot]

@AdamLindenthal said: Hi bodewig, correct - you cannot unfortunatelly attach files to the issue.

But we can - next time just drop a link to github project or whatever (e.g. you can send it via email to one of us after the communication starts here) and we will attach it for you.

Regarding the problem you are experiencing - this needs to be tested and investigated - thus I am moving it to backlog.

Thanks, Adam

PS: should you still have something more to attach here, feel free to drop a link or send a zip to me.

[glassfishrobot]

@AdamLindenthal said: Fixed in 2.17.

[glassfishrobot]

defnull said: The provided test fails again in 2.25.1

[glassfishrobot]

@pavelbucek said: And how is that different compared to https://github.com/jersey/jersey/blob/df7efb6d0139af64fe9bbd4818d82eaf8b6db414/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/server/RequestScopedAndAsyncTest.java ?

[glassfishrobot]

defnull said: I am currently trying to find that out.

In my current project, I noticed that in async mode (triggered by @Suspended) neither Factory<Injectable>.dispose(...) is called for @RequestScope injections, nor ContainerResponseFilter.filter(...) is triggered after the request. I then literally copy&pasted the test case from ##2949 into a fresh project, run it and it failed. That's all I have tested so far.

I'm waiting for jersey to build and then try to run the test you mentioned. If that does not fail, then there must be something fishy with my eclipse setup. Wouldn't be the first time.

[glassfishrobot]

defnull said: False alarm. The test provided by @bodewig is timing-sensitive and may fail simply because the thread responsible for calling dispose() is not fast enough. The test in the jersey repository does the right thing and waits up to 500ms for the thread to catch up. The tests in my own project failed for the same reason.

Sorry for the noise.

[glassfishrobot]

This issue was imported from java.net JIRA JERSEY-2677

[glassfishrobot]

Marked as fixed on Thursday, February 19th 2015, 7:07:37 am