[Servlet] Filter threadsafe maken

[JKVA]

Voor mijn huidige project moeten we zorgen dat na het inloggen altijd een speciale pagina getoond wordt, waarmee wat gegevens in de sessie gezet kunnen worden. Die worden namelijk door de rest van de applicatie gebruikt.

Om dit op te lossen hebben we een Filter gemaakt welke op een dummy sessievariabele controleert en afhankelijk van of de variabele bestaat, wordt er al dan niet doorgestuurd naar die speciale homepage.

Java:

/** {@inheritDoc} */
public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException {
    HttpServletRequest req = (HttpServletRequest) request;
    HttpServletResponse res = (HttpServletResponse) response;
    HttpSession ses = req.getSession();

    if (excludeThisView(req, res)) {
        chain.doFilter(req, res);
    } else {
        /*
         * Synchronizeren op het sessie object. De sessie wijst bij
         * verschillende requests naar exact hetzelfde object.
         */
        synchronized (ses) {
            if (ses.getAttribute(DUMMY_SESSION_STRING) == null) {
                ses.setAttribute(DUMMY_SESSION_STRING, new Object());

                String ctxRoot = req.getContextPath();
                res.sendRedirect(ctxRoot + HOMEPAGE);
                return;
            }
        }

        chain.doFilter(req, res);
    }
}

Nu is naar mijn idee één van de kernpunten van het servletframework dat het multi threaded is en dat wil ik in zo'n klein stom filtertje niet om zeep helpen. Vandaar dat de chain.doFilter() call niet in een synchronized context staat.

Ik sync op de session, want de state is steeds gekoppeld aan een specifieke thread. Meerdere threads hebben namelijk een andere session. Over meerdere requests wordt hetzelfde session object gebruikt, dat heb ik al vrij provisorisch gecontroleerd.

Tot zover mijn redeneringen en argumenten. Ik ben benieuwd naar jullie opbouwende (hoeft niet ) feedback.

[Verwijderd]

Synchroniseren op HttpSession heeft weinig zin. Er wordt namelijk doorgaans een facade terug gegeven En die facade hoeft echt niet dezelfde te zijn voor verschillende requests. Je kunt wel op de sessie key en dan String.intern() syncen maar ook dat is niet waterdicht. Je kunt namelijk meerdere JVM's gebruiken en dan heeft ook dat weinig zin. Maar als je dus 1 jvm / applicatie server gebruikt dan kun je dus synchroniseren op HttpSession.getId().intern().

[ Voor 26% gewijzigd door Verwijderd op 26-06-2007 11:20 ]

[JKVA]

Verwijderd schreef op dinsdag 26 juni 2007 @ 11:17:
Synchroniseren op HttpSession heeft weinig zin. Er wordt namelijk doorgaans een facade terug gegeven En die facade hoeft echt niet dezelfde te zijn voor verschillende requests. Je kunt wel op de sessie key en dan String.intern() syncen maar ook dat is niet waterdicht. Je kunt namelijk meerdere JVM's gebruiken en dan heeft ook dat weinig zin. Maar als je dus 1 jvm / applicatie server gebruikt dan kun je dus synchroniseren op HttpSession.getId().intern().

Hmm, ik zie idd dat er wat platform dependent dingen in HttpSession staan. De Session is idd een Facade, maar wel steeds dezelfde instantie, maar blijkbaar ligt dat dus aan mijn JBoss/Tomcat.

Die getId returnt een String en daar wil je niet op syncen. Ik neem aan dat die intern methode ervoor zorgt dat ik dat wel kan doen?

[Verwijderd]

JKVA schreef op dinsdag 26 juni 2007 @ 11:26:
Hmm, ik zie idd dat er wat platform dependent dingen in HttpSession staan. De Session is idd een Facade, maar wel steeds dezelfde instantie, maar blijkbaar ligt dat dus aan mijn JBoss/Tomcat.

Die getId returnt een String en daar wil je niet op syncen. Ik neem aan dat die intern methode ervoor zorgt dat ik dat wel kan doen?

De id String die wordt teruggeven is simpelweg een session key maar dat hoeft uiteraard niet dezelfde String instantie te zijn. intern() zorgt er voor dat je de String instantie uit de String pool pakt (of op dat moment toevoegd). Je hebt dan dus altijd hetzelfde object te pakken binnen dezelfde jvm instantie.

edit:
Overigens zal in praktijk je code hoogstwaarschijnlijk gewoon werken (mits je je dus beperkt tot 1 instantie). Maarja, het is weer zo'n dingetje dat nergens in de specs wordt afgedwongen dus kun je het formeel niet doen. Dus hoe pragmatisch ben je

[ Voor 14% gewijzigd door Verwijderd op 26-06-2007 11:35 ]

[Confusion]

Ik gebruik in een soortgelijk geval een lock object in de sessie, die als guard dient voor een specifiek attribuut. Dat moet wel glashelder gedocumenteerd ziin voor andere developers (mompelt /me, terwijl hij naar de klasse bladert om de documentatie aan te vullen ...)

[Verwijderd]

Confusion schreef op dinsdag 26 juni 2007 @ 11:35:
Ik gebruik in een soortgelijk geval een lock object in de sessie, die als guard dient voor een specifiek attribuut. Dat moet wel glashelder gedocumenteerd ziin voor andere developers (mompelt /me, terwijl hij naar de klasse bladert om de documentatie aan te vullen ...)

Is niet waterdicht, of niet 'waterdichter' dan getId().intern(). Je moet immers het lock object in de sessie zetten en daarbij ontkom je er niet aan dat je moet synchroniseren op applicatie niveau. En dat maakt dus dat je op eenzelfde lock niveau zit.

--hmmz, hier stond onzin---

[ Voor 38% gewijzigd door Verwijderd op 26-06-2007 11:56 ]

[JKVA]

Verwijderd schreef op dinsdag 26 juni 2007 @ 11:51:
[...]
Is niet waterdicht, of niet 'waterdichter' dan getId().intern(). Je moet immers het lock object in de sessie zetten en daarbij ontkom je er niet aan dat je moet synchroniseren op applicatie niveau. En dat maakt dus dat je op eenzelfde lock niveau zit.

--hmmz, hier stond onzin---

Lol, doel je met onzin op:
"Is niet waterdicht, of niet 'waterdichter' dan getId().intern(). Je moet immers het lock object in de sessie zetten en daarbij ontkom je er niet aan dat je moet synchroniseren op applicatie niveau. En dat maakt dus dat je op eenzelfde lock niveau zit."
?
Of op iets wat er nu niet meer staat?

[Verwijderd]

Iets wat er nu niet meer staat

[misfire]

Ik weet niet zeker of ik de bedoeling snap. De Session syncen voor attribute access is onnodig: dit wordt door de container geregeld. Als het de bedoeling is hier op de een of andere manier een flow af te dwingen dan klopt het niet. De sendRedirect stuurt namelijk een 300 code terug en daarna wordt de lock released, vóórdat die nieuwe pagina dus ook maar aangeroepen is. De synchronized voegt dus helemaal niets toe in dit geval.

De opmerkingen die ik verder heb:
* Object is niet serializable, waardoor session replication niet meer werkt. Handiger is om een String of Boolean oid te gebruiken als dummy waarde.
* Als je redirect naar een bepaalde beginpagina met het doel dat daar eerst een waarde ingevuld wordt, dan zou ik op die waarde testen, ipv dummy. Op deze manier kan een gebruiker simpelweg twee keer klikken of back oid te doen om alsnog zonder invullen van extra info verder te gaan.

[Verwijderd]

misfire schreef op dinsdag 26 juni 2007 @ 16:18:
De sendRedirect stuurt namelijk een 300 code terug en daarna wordt de lock released, vóórdat die nieuwe pagina dus ook maar aangeroepen is. De synchronized voegt dus helemaal niets toe in dit geval.

Wat nu als er twee requests gelijktijdig wordt afgevuurd door dezelfde client. Bijvoorbeeld omdat we het niet over een html page hebben maar twee images op een page. Dan kan wel degelijk de situatie zich voordoen dat er twee keer geredirect wordt.

UIteraard los van de hypothetische use-case die daar voor nodig is

[misfire]

Verwijderd schreef op dinsdag 26 juni 2007 @ 20:31:
[...]
Wat nu als er twee requests gelijktijdig wordt afgevuurd door dezelfde client. Bijvoorbeeld omdat we het niet over een html page hebben maar twee images op een page. Dan kan wel degelijk de situatie zich voordoen dat er twee keer geredirect wordt.

UIteraard los van de hypothetische use-case die daar voor nodig is

Tja in dat geval zou het enig nut hebben vanwege de minuscule race conditie, en dan zou je nog jouw truc nodig hebben van een interned session id, omdat een session niet gegarandeerd altijd dezelfde object instantie is (en gaat dus ook stuk op een cluster). Als de sessie nieuw is (eerste verzoek) dan werkt zelfs dat niet, omdat ook het tweede concurrent verzoek een nieuwe sessie is (er is dan immers nog geen client cookie/url om te correleren) en dus ook een ander session id heeft.

Ik bedacht me net dat het waarschijnlijk makkelijker is om de isNew() methode van session te gebruiken voor dit probleem, nu ik in de API docs van HttpSession kijk zie ik zelfs al een voorbeeld.

[flowerp]

Verwijderd schreef op dinsdag 26 juni 2007 @ 11:17:
Synchroniseren op HttpSession heeft weinig zin.

Is dat zo?

Kijk eens voor de grap naar welke code jouw jsp engine (bv Jasper in Tomcat) genereert voor de standaard usebean tag met session scope.

Volgende de Java EE spec mag deze namelijk maar door 1 thread en 1 malig worden aangemaakt als er nog geen instantie reeds in de sessie van voorkomt. Iniedergeval van Tomcat en Orion weet ik dat in de servlet die voor de JSP gegenereerd wordt gewoon een synchronized (session) staat.

[Verwijderd]

flowerp schreef op woensdag 27 juni 2007 @ 00:24:
Volgende de Java EE spec mag deze namelijk maar door 1 thread en 1 malig worden aangemaakt als er nog geen instantie reeds in de sessie van voorkomt. Iniedergeval van Tomcat en Orion weet ik dat in de servlet die voor de JSP gegenereerd wordt gewoon een synchronized (session) staat.

Zou kunnen, dat weet ik niet. In ieder geval is dat dan een container specifieke implementatie. De container moet het gedrag garanderen, hoe dat geimplementeerd is gaat daar aan voorbij. Maar zoals eerder gezegd: "hoe pragmatisch ben je." Het zal in de praktijk wel zo zijn dat die kans van 1 op een miljard*miljard niet optreedt. En als het hier mis gaat: "who cares"

[JKVA]

misfire schreef op dinsdag 26 juni 2007 @ 16:18:
De opmerkingen die ik verder heb:
* Object is niet serializable, waardoor session replication niet meer werkt. Handiger is om een String of Boolean oid te gebruiken als dummy waarde.

Idd, goed punt.

* Als je redirect naar een bepaalde beginpagina met het doel dat daar eerst een waarde ingevuld wordt, dan zou ik op die waarde testen, ipv dummy. Op deze manier kan een gebruiker simpelweg twee keer klikken of back oid te doen om alsnog zonder invullen van extra info verder te gaan.

Ja, klopt, maar in dit geval speelt dat niet. Het is vooral bedoeld om te zorgen dat mensen een homepage hebben en niet middenin de applicatie kunnen stappen, want die is daar niet op berekend.

misfire schreef op dinsdag 26 juni 2007 @ 23:54:
[...]
Ik bedacht me net dat het waarschijnlijk makkelijker is om de isNew() methode van session te gebruiken voor dit probleem, nu ik in de API docs van HttpSession kijk zie ik zelfs al een voorbeeld.

Ik zal eens kijken. Het oogt iig al goed.

offtopic:
Moet je dit voor SCWCD ook weten? Dan is het misschien toch de moeite om het een keer te doen.

Verwijderd schreef op woensdag 27 juni 2007 @ 07:18:
[...]
Maar zoals eerder gezegd: "hoe pragmatisch ben je." Het zal in de praktijk wel zo zijn dat die kans van 1 op een miljard*miljard niet optreedt. En als het hier mis gaat: "who cares"

Tja, het was meer zoeken naar een eenvoudige, maar toch dichte oplossing. De kans op een race condition (precies na de if) is idd miniem, maar ook aan de voorkant heb ik liever geen stack traces en zo (zelfs foutpagina's), alleen maar omder de gebruiker een beetje gek doet. Komt een beetje knullig over (zeker aangezien ik nu voor een zeer grote organisatie bezig ben, dan telt het in mijn optiek nog erger)

[Confusion]

JKVA schreef op woensdag 27 juni 2007 @ 10:28:
maar ook aan de voorkant heb ik liever geen stack traces

offtopic:
Die kan je, in ieder geval in Tomcat, in de server.xml uitzetten, zodat een gebruiker nooit een stacktrace te zien krijgt.

[JKVA]

Confusion schreef op woensdag 27 juni 2007 @ 10:53:
[...]

offtopic:
Die kan je, in ieder geval in Tomcat, in de server.xml uitzetten, zodat een gebruiker nooit een stacktrace te zien krijgt.

Ja dat begrijp ik, maar ik doel erop dat de applicatie er gewoon goed mee overweg gaat, in plaats van de gevolgen te verdoezelen. (wat ook moet gebeuren)

[Verwijderd]

JKVA schreef op woensdag 27 juni 2007 @ 11:00:
Ja dat begrijp ik, maar ik doel erop dat de applicatie er gewoon goed mee overweg gaat, in plaats van de gevolgen te verdoezelen. (wat ook moet gebeuren)

Helemaal mee eens. Ik zie alleen niet hoe dat in dit specifieke geval tot stacktraces leidt.

[JKVA]

Verwijderd schreef op woensdag 27 juni 2007 @ 11:10:
[...]
Helemaal mee eens. Ik zie alleen niet hoe dat in dit specifieke geval tot stacktraces leidt.

Nou, als er iets misgaat in dit Filter door een race condition dan kan -als ik het goed heb- de gebruiker in een andere pagina dan de homepage belanden. En als die andere pagina aannames doet over session state, dan kan het misgaan. Gevolg: stack traces of wat voor foutrapportage dan ook...

[Verwijderd]

Ah okee, dus je bent eigenlijk opzoek naar een manier waarop je eenmalig een hoop user gerelateerde data de sessie in wil pompen. Begrijp ik dat goed?

In dat geval zou ik namelijk helemaal niet synchroniseren en voor lief nemen dat hooguit dergelijke sessie data twee keer wordt opgevraagd.

[JKVA]

Verwijderd schreef op woensdag 27 juni 2007 @ 11:29:
Ah okee, dus je bent eigenlijk opzoek naar een manier waarop je eenmalig een hoop user gerelateerde data de sessie in wil pompen. Begrijp ik dat goed?

In dat geval zou ik namelijk helemaal niet synchroniseren en voor lief nemen dat hooguit dergelijke sessie data twee keer wordt opgevraagd.

Eigenlijk wil ik een standaard homepage waar je altijd in begint. Dan kan ik flow 'afdwingen' en gaat alles goed, ook na een sessie timeout o.i.d.

Momenteel heb ik de simpele optie gebruikt, dus zonder extra synchronisatie, maar dit is niet de eerste keer dat ik zo'n filter maak voor dit doeleinde, dus ik wilde er eens goed over nadenken om het goed waterdicht te maken.

Dus eigenlijk een soort uitbreiding op de J2EE spec, namelijk een na-inlog-homepage.

[Verwijderd]

Ik snap nog niet helemaal welke flow je exact wilt bereiken, komt het in essentie op het volgende neer:

Wel aangemeld:
browsen naar 'home' -> redirect naar 'aangemeld home'


Niet aangemeld:
browsen naar 'home' -> redirect naar 'aanmelden pagina'
browsen naar 'aangemeld home' -> redirect naar 'aanmelden pagina'

Zo iets?

[JKVA]

Verwijderd schreef op woensdag 27 juni 2007 @ 13:47:
Ik snap nog niet helemaal welke flow je exact wilt bereiken, komt het in essentie op het volgende neer:

Wel aangemeld:
browsen naar 'home' -> redirect naar 'aangemeld home'


Niet aangemeld:
browsen naar 'home' -> redirect naar 'aanmelden pagina'
browsen naar 'aangemeld home' -> redirect naar 'aanmelden pagina'

Zo iets?

Het staat in principe los van het inloggen. Misschien een verkeerde woordkeuze. Mijn doel is in feite dat je niet op een willekeurige plek in de applicatie in kunt prikken, bijvoorbeeld omdat je daar een bookmark gemaakt hebt. Dan kan namelijk voor fouten zorgen, dus wil ik de gebruiker langs een vaste flow leiden, namelijk beginnende op de homepage en dan gewoon netjes de links en knoppen volgen, zoals we bedoeld hebben. Anders gaat het fout. (deze applicatie is namelijk totaal niet ReST)

Dus:
- Eerste bezoek ==> redirect homepage
- Tweede bezoek en daarna ==> gewoon de opgevraagde resource

Inloggen staat er los van, dat komt altijd eerst.

[Confusion]

Dan controleer je toch gewoon of iemand de site al eerder beziocht heeft (middels cookie of login) en stuurt de persoon daarna door?

Als iemand aan het rotzooien is, dan heeft'ie pech; je kan niet iedere dodo precies zo sturen als je wilt. De rest van je applicatie moet evengoed bestand zijn tegen mensen die de flow niet netjes volgen; anders vind iemand wel een manier om de applicatie te misbruiken door de flow te omzeilen.

[JKVA]

Mja, misschien draaf ik wel door. De applicatie zelf staat los van de flow en weet daar dus niets van. Het wordt alleen een applicatie voor op het internet en dan gaat er bij mij een knop om dat alles retestrak moet zijn, zelfs als iemand wat raars doet.