You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@cxf.apache.org by Gina Choi <gi...@gmail.com> on 2012/10/12 17:04:21 UTC
Fediz(SSO) - Do we need to unset token?
Hi Apache CXF team,
I would like to consult with you if it is something that I worry about. I
have heard that when ue threadpool, we need to clean up ThreadLocals that we
set to prevent potential PermGen spance issue. Currently our application is
using threadpool.
I am looking at org.apache.cxf.fediz.example.SecurityTokenThreadLocal.java
class and I only see set() and get() method. Do I need to unset the token at
some point?
Thanks in advance.
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524.html
Sent from the cxf-user mailing list archive at Nabble.com.
RE: Fediz(SSO) - Do we need to unset token?
Posted by Oliver Wulff <ow...@talend.com>.
agreed. Will fix this for 1.0.2.
------
Oliver Wulff
Blog: http://owulff.blogspot.com
Solution Architect
http://coders.talend.com
Talend Application Integration Division http://www.talend.com
________________________________________
From: Daniel Kulp [dkulp@apache.org]
Sent: 15 October 2012 18:55
To: users@cxf.apache.org
Subject: Re: Fediz(SSO) - Do we need to unset token?
On Oct 15, 2012, at 12:41 PM, Gina Choi <gi...@gmail.com> wrote:
> Hi Dan,
>
> Thanks for your response. If we look at
> org.apache.cxf.fediz.example.SecurityTokenThreadLocal.java class, I only see
> get and set method. I could directly call TLS.remove() to unset the token,
> but should we have a remove method inside SecurityTokenThreadLocal.java
> class?
Most likely yes. In addition, I would recommend that the set method be changed to:
if (token == null) {
TLS.remove();
} else {
TLS.set(token);
}
Dan
>
>
> public final class SecurityTokenThreadLocal {
>
> private static final ThreadLocal<Element> TLS =
> new ThreadLocal<Element>() {
> };
>
> private SecurityTokenThreadLocal() {
> }
>
> public static void setToken(Element token) {
> TLS.set(token);
> }
>
> public static Element getToken() {
> return TLS.get();
> }
>
>
> }
>
> Gina
>
>
>
> --
> View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716675.html
> Sent from the cxf-user mailing list archive at Nabble.com.
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com
RE: Fediz(SSO) - Do we need to unset token?
Posted by Gina Choi <gi...@gmail.com>.
Thanks Oliver. We look forward to meeting Fediz1.1 release.
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716952.html
Sent from the cxf-user mailing list archive at Nabble.com.
RE: Fediz(SSO) - Do we need to unset token?
Posted by Oliver Wulff <ow...@talend.com>.
The Servlet Filter adds the token to the TLS and therefore, he should also remove it before the response is sent back.
I'm planning to create a fediz-cxf module which contains this kind of functionality in fediz 1.1
Oli
------
Oliver Wulff
Blog: http://owulff.blogspot.com
Solution Architect
http://coders.talend.com
Talend Application Integration Division http://www.talend.com
________________________________________
From: Gina Choi [ginachoi88@gmail.com]
Sent: 16 October 2012 21:38
To: users@cxf.apache.org
Subject: Re: Fediz(SSO) - Do we need to unset token?
Hi Dan,
Thanks for taking time to review this issue. My next questions is when is
right time to remove the token. If I remove the token right after user is
authenticated, will it affect renewing token later? Or do we need to remove
token before invalidate session for the user?
Thanks.
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716769.html
Sent from the cxf-user mailing list archive at Nabble.com.
Re: Fediz(SSO) - Do we need to unset token?
Posted by Gina Choi <gi...@gmail.com>.
Hi Dan,
Thanks for taking time to review this issue. My next questions is when is
right time to remove the token. If I remove the token right after user is
authenticated, will it affect renewing token later? Or do we need to remove
token before invalidate session for the user?
Thanks.
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716769.html
Sent from the cxf-user mailing list archive at Nabble.com.
Re: Fediz(SSO) - Do we need to unset token?
Posted by Daniel Kulp <dk...@apache.org>.
On Oct 16, 2012, at 10:27 AM, Daniel Kulp <dk...@apache.org> wrote:
>> Hi Dan,
>>
>> Thanks for your tip. Could you explain to me why do we remove the token when
>> it is null? I don't know much about ThreadLocal.
>
> Internally, the "Thread" object keeps a map of the ThreadLocal object -> Value. If you set the value to null, it's still in the map with a null value. However, if you call remove, it removes the entire thing from the map which helps make sure there are no threads holding onto anything that could cause jar locking and such.
>
> It's similar to HashMap behavior. With a HashMap, you can do:
>
> hashMap.set("foo", null);
> or
> hashMap.remove("foo")
>
> In both cases, a hashMap.get("foo") would return null. However, the first method would result in a size()==1 has there is a key/value pair in the map. In the second case, the map is really empty.
I actually just realized why this is even more critical for Fediz. With the ThreadLocal, the "key" in the map is the actual ThreadLocal object. Thus, if it's not removed from the map entirely, if the ThreadLocal object holds something strongly, it can lock thing. In many cases, this isn't super critical as ThreadLocal.class is loaded by the system classloader so if the value is null, it doesn't really "leak" anything other than a space in the map.
However, in Fediz, we're doing:
private static final ThreadLocal<Element> TLS =
new ThreadLocal<Element>() {
};
The { }; on the new ThreadLocal is causing an anonymous subclass of ThreadLocal to be created. That is then loaded by the Fediz class loader. If that thread local is left in the map, it can lock the class loader used to load all the Fediz stuff and thus cause perm gen leaks and such. Thus, I'd recommend:
1) Make sure we call remove() when appropriate
2) Remove the { }; from the above to make it not be an anonymous class.
Dan
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com
Re: Fediz(SSO) - Do we need to unset token?
Posted by Daniel Kulp <dk...@apache.org>.
On Oct 16, 2012, at 10:16 AM, Gina Choi <gi...@gmail.com> wrote:
> Hi Dan,
>
> Thanks for your tip. Could you explain to me why do we remove the token when
> it is null? I don't know much about ThreadLocal.
Internally, the "Thread" object keeps a map of the ThreadLocal object -> Value. If you set the value to null, it's still in the map with a null value. However, if you call remove, it removes the entire thing from the map which helps make sure there are no threads holding onto anything that could cause jar locking and such.
It's similar to HashMap behavior. With a HashMap, you can do:
hashMap.set("foo", null);
or
hashMap.remove("foo")
In both cases, a hashMap.get("foo") would return null. However, the first method would result in a size()==1 has there is a key/value pair in the map. In the second case, the map is really empty.
Dan
>
> I was thinking the following.
>
> if(token != null){
> TLS.set(token)
> }
>
> Gina
>
>
>
> --
> View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716743.html
> Sent from the cxf-user mailing list archive at Nabble.com.
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com
Re: Fediz(SSO) - Do we need to unset token?
Posted by Gina Choi <gi...@gmail.com>.
Hi Dan,
Thanks for your tip. Could you explain to me why do we remove the token when
it is null? I don't know much about ThreadLocal.
I was thinking the following.
if(token != null){
TLS.set(token)
}
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716743.html
Sent from the cxf-user mailing list archive at Nabble.com.
Re: Fediz(SSO) - Do we need to unset token?
Posted by Daniel Kulp <dk...@apache.org>.
On Oct 15, 2012, at 12:41 PM, Gina Choi <gi...@gmail.com> wrote:
> Hi Dan,
>
> Thanks for your response. If we look at
> org.apache.cxf.fediz.example.SecurityTokenThreadLocal.java class, I only see
> get and set method. I could directly call TLS.remove() to unset the token,
> but should we have a remove method inside SecurityTokenThreadLocal.java
> class?
Most likely yes. In addition, I would recommend that the set method be changed to:
if (token == null) {
TLS.remove();
} else {
TLS.set(token);
}
Dan
>
>
> public final class SecurityTokenThreadLocal {
>
> private static final ThreadLocal<Element> TLS =
> new ThreadLocal<Element>() {
> };
>
> private SecurityTokenThreadLocal() {
> }
>
> public static void setToken(Element token) {
> TLS.set(token);
> }
>
> public static Element getToken() {
> return TLS.get();
> }
>
>
> }
>
> Gina
>
>
>
> --
> View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716675.html
> Sent from the cxf-user mailing list archive at Nabble.com.
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com
Re: Fediz(SSO) - Do we need to unset token?
Posted by Gina Choi <gi...@gmail.com>.
Hi Dan,
Thanks for your response. If we look at
org.apache.cxf.fediz.example.SecurityTokenThreadLocal.java class, I only see
get and set method. I could directly call TLS.remove() to unset the token,
but should we have a remove method inside SecurityTokenThreadLocal.java
class?
public final class SecurityTokenThreadLocal {
private static final ThreadLocal<Element> TLS =
new ThreadLocal<Element>() {
};
private SecurityTokenThreadLocal() {
}
public static void setToken(Element token) {
TLS.set(token);
}
public static Element getToken() {
return TLS.get();
}
}
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716675.html
Sent from the cxf-user mailing list archive at Nabble.com.
Re: Fediz(SSO) - Do we need to unset token?
Posted by Daniel Kulp <dk...@apache.org>.
On Oct 15, 2012, at 9:32 AM, Gina Choi <gi...@gmail.com> wrote:
> The reason that I am checking this is because, we are getting memory leak
> message when stop Tomcat(7.0.28) due to threadlocal "failed to remove" error
> message. I am not sure if it is fake warning or real. Another VM uses same
> tomcat version, but doesn't report errors. I am trying to figure this out.
They definitely should be cleared when the thread unwinds. It should be a call to ThreadLocal.remove().
Dan
>
> Thanks.
>
> Gina
>
>
>
> --
> View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716650.html
> Sent from the cxf-user mailing list archive at Nabble.com.
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com
Re: Fediz(SSO) - Do we need to unset token?
Posted by Gina Choi <gi...@gmail.com>.
The reason that I am checking this is because, we are getting memory leak
message when stop Tomcat(7.0.28) due to threadlocal "failed to remove" error
message. I am not sure if it is fake warning or real. Another VM uses same
tomcat version, but doesn't report errors. I am trying to figure this out.
Thanks.
Gina
--
View this message in context: http://cxf.547215.n5.nabble.com/Fediz-SSO-Do-we-need-to-unset-token-tp5716524p5716650.html
Sent from the cxf-user mailing list archive at Nabble.com.