You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ja...@apache.org on 2009/05/01 19:47:52 UTC
svn commit: r770771 - in /ofbiz/trunk:
applications/securityext/src/org/ofbiz/securityext/test/
framework/security/src/org/ofbiz/security/authz/
framework/webapp/src/org/ofbiz/webapp/control/
Author: jaz
Date: Fri May 1 17:47:52 2009
New Revision: 770771
URL: http://svn.apache.org/viewvc?rev=770771&view=rev
Log:
Often thread pools do not clear ThreadLocal, implemented a workaround to handle this
Modified:
ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java
ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java
ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
Modified: ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java?rev=770771&r1=770770&r2=770771&view=diff
==============================================================================
--- ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java (original)
+++ ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java Fri May 1 17:47:52 2009
@@ -4,6 +4,7 @@
import org.ofbiz.base.util.Debug;
import org.ofbiz.security.SecurityConfigurationException;
+import org.ofbiz.security.authz.AbtractAuthorization;
import org.ofbiz.security.authz.Authorization;
import org.ofbiz.security.authz.AuthorizationFactory;
import org.ofbiz.service.testtools.OFBizTestCase;
@@ -11,7 +12,7 @@
public class AuthorizationTests extends OFBizTestCase {
private static final String module = AuthorizationTests.class.getName();
- protected Authorization security;
+ protected Authorization security = null;
public AuthorizationTests(String name) {
super(name);
@@ -19,7 +20,10 @@
@Override
public void setUp() throws SecurityConfigurationException {
- security = AuthorizationFactory.getInstance(delegator);
+ if (security == null) {
+ security = AuthorizationFactory.getInstance(delegator);
+ }
+ AbtractAuthorization.clearThreadLocal();
}
public void testBasicAdminPermission() throws Exception {
Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java?rev=770771&r1=770770&r2=770771&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java (original)
+++ ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java Fri May 1 17:47:52 2009
@@ -125,9 +125,11 @@
}
// set the tracking values on thread local
+ boolean initialCall = false;
if (UtilValidate.isEmpty(threadUid)) {
origPermission.set(permission);
uid.set(userId);
+ initialCall = true;
}
// split the permission string; so we can walk up the levels
@@ -171,7 +173,7 @@
// finally check dynamic permission (outside the loop)
String threadPerm = origPermission.get();
- if (!permission.equals(threadPerm)) {
+ if (initialCall || !permission.equals(threadPerm)) {
if (hasDynamicPermission(userId, expandedPermission, context)) {
// permission granted
handleAutoGrantPermissions(userId, expandedPermission, context);
@@ -207,4 +209,14 @@
autoGrant.set(granted);
}
}
+
+ /**
+ * Used to clear the values set in ThreadLocal
+ * -- needed when thread pools are used which do not handle clearing between requests
+ */
+ public static void clearThreadLocal() {
+ origPermission.remove();
+ autoGrant.remove();
+ uid.remove();
+ }
}
Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java?rev=770771&r1=770770&r2=770771&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java Fri May 1 17:47:52 2009
@@ -54,6 +54,7 @@
import org.ofbiz.security.Security;
import org.ofbiz.security.SecurityConfigurationException;
import org.ofbiz.security.SecurityFactory;
+import org.ofbiz.security.authz.AbtractAuthorization;
import org.ofbiz.security.authz.Authorization;
import org.ofbiz.security.authz.AuthorizationFactory;
import org.ofbiz.service.GenericDispatcher;
@@ -129,6 +130,9 @@
Thread.currentThread().setContextClassLoader(localCachedClassLoader);
}
+ // reset thread local security; used when thread pools don't clear
+ AbtractAuthorization.clearThreadLocal();
+
// set the webSiteId in the session
httpRequest.getSession().setAttribute("webSiteId", config.getServletContext().getAttribute("webSiteId"));
ThreadLocal (was re: svn commit: r770771)
Posted by Andrew Zeneski <an...@hotwaxmedia.com>.
Adam,
Another option which might be better/easier/less intrusive (or very
similar) is to create an single place for storing thread local
variables. Like an OFBizThreadLocal class, which extends ThreadLocal.
Keep all the variables inside this class, then just tie in
ContextFilter (after the final chain.doFilter() call) and in the
JobInvoker class calls to OFBizThreadLocal.clear().
What do you think about that?
Andrew
On May 1, 2009, at 3:00 PM, andrew.zeneski@hotwaxmedia.com wrote:
> Yeah I'm game for a more definite fix. But what about app server
> threads?
>
> Andrew
>
>
> On May 1, 2009, at 2:42 PM, Adam Heath <do...@brainfood.com> wrote:
>
>> jaz@apache.org wrote:
>>> Author: jaz
>>> Date: Fri May 1 17:47:52 2009
>>> New Revision: 770771
>>>
>>> URL: http://svn.apache.org/viewvc?rev=770771&view=rev
>>> Log:
>>> Often thread pools do not clear ThreadLocal, implemented a
>>> workaround to handle this
>>
>> Actually, never, until the Thread is shutdown. ThreadLocal is just
>> for storing stuff against a thread-type key. What you want is a
>> PoolThreadLocal, which doesn't exist.
>>
>> I guess I could add code to support the same thing that webslinger
>> does for this case. It would require modifying ControlServlet,
>> JobPoller, and any other pool-like container class, to add a hook to
>> run an AtExit list of hooks. Then, add a utility class that allows
>> for singleton per-thread calls, and at-exit calls when the pool
>> returns the thread for further processing.
>>
>> If this sounds confusing, it's that it's difficult for me to explain,
>> and would just be easier if I add the feature(or otherwise show the
>> code).
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/ framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adam Heath" <do...@brainfood.com>
To: <de...@ofbiz.apache.org>
Sent: Friday, May 01, 2009 10:52 PM
Subject: Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/
framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
> andrew.zeneski@hotwaxmedia.com wrote:
>> Yeah I'm game for a more definite fix. But what about app server threads?
>
> At some point, the app server will call into ofbiz code. This code,
> in all likely hood, is restricted to a few select places(ie, like the
> ControlServlet is run by a jetty or catalina container). This code is
> what needs to be modified to run the post-thread cleanup.
>
> Any other code that actually *does* maintain it's own thread pool
> would also be modified with this hook.
>
> Have you looked at the Executor framework?
Executor framework : interesting, I was not aware !
Thanks
Jacques
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/ framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by Jacques Le Roux <ja...@les7arts.com>.
I think we have currently some issues pending in Jira that would benefit of a such effort (look for Mouawad in Jira)
Jacques
From: "Andrew Zeneski" <an...@hotwaxmedia.com>
> Yeah, I've always wanted to change the service engine to use that...
>
> On May 1, 2009, at 4:52 PM, Adam Heath wrote:
>
>> andrew.zeneski@hotwaxmedia.com wrote:
>>> Yeah I'm game for a more definite fix. But what about app server
>>> threads?
>>
>> At some point, the app server will call into ofbiz code. This code,
>> in all likely hood, is restricted to a few select places(ie, like the
>> ControlServlet is run by a jetty or catalina container). This code is
>> what needs to be modified to run the post-thread cleanup.
>>
>> Any other code that actually *does* maintain it's own thread pool
>> would also be modified with this hook.
>>
>> Have you looked at the Executor framework?
>
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/ framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by Andrew Zeneski <an...@hotwaxmedia.com>.
Yeah, I've always wanted to change the service engine to use that...
On May 1, 2009, at 4:52 PM, Adam Heath wrote:
> andrew.zeneski@hotwaxmedia.com wrote:
>> Yeah I'm game for a more definite fix. But what about app server
>> threads?
>
> At some point, the app server will call into ofbiz code. This code,
> in all likely hood, is restricted to a few select places(ie, like the
> ControlServlet is run by a jetty or catalina container). This code is
> what needs to be modified to run the post-thread cleanup.
>
> Any other code that actually *does* maintain it's own thread pool
> would also be modified with this hook.
>
> Have you looked at the Executor framework?
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/
framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by Adam Heath <do...@brainfood.com>.
andrew.zeneski@hotwaxmedia.com wrote:
> Yeah I'm game for a more definite fix. But what about app server threads?
At some point, the app server will call into ofbiz code. This code,
in all likely hood, is restricted to a few select places(ie, like the
ControlServlet is run by a jetty or catalina container). This code is
what needs to be modified to run the post-thread cleanup.
Any other code that actually *does* maintain it's own thread pool
would also be modified with this hook.
Have you looked at the Executor framework?
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/ framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by an...@hotwaxmedia.com.
Yeah I'm game for a more definite fix. But what about app server
threads?
Andrew
On May 1, 2009, at 2:42 PM, Adam Heath <do...@brainfood.com> wrote:
> jaz@apache.org wrote:
>> Author: jaz
>> Date: Fri May 1 17:47:52 2009
>> New Revision: 770771
>>
>> URL: http://svn.apache.org/viewvc?rev=770771&view=rev
>> Log:
>> Often thread pools do not clear ThreadLocal, implemented a
>> workaround to handle this
>
> Actually, never, until the Thread is shutdown. ThreadLocal is just
> for storing stuff against a thread-type key. What you want is a
> PoolThreadLocal, which doesn't exist.
>
> I guess I could add code to support the same thing that webslinger
> does for this case. It would require modifying ControlServlet,
> JobPoller, and any other pool-like container class, to add a hook to
> run an AtExit list of hooks. Then, add a utility class that allows
> for singleton per-thread calls, and at-exit calls when the pool
> returns the thread for further processing.
>
> If this sounds confusing, it's that it's difficult for me to explain,
> and would just be easier if I add the feature(or otherwise show the
> code).
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/ framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adam Heath" <do...@brainfood.com>
> jaz@apache.org wrote:
>> Author: jaz
>> Date: Fri May 1 17:47:52 2009
>> New Revision: 770771
>>
>> URL: http://svn.apache.org/viewvc?rev=770771&view=rev
>> Log:
>> Often thread pools do not clear ThreadLocal, implemented a workaround to handle this
>
> Actually, never, until the Thread is shutdown. ThreadLocal is just
> for storing stuff against a thread-type key. What you want is a
> PoolThreadLocal, which doesn't exist.
>
> I guess I could add code to support the same thing that webslinger
> does for this case. It would require modifying ControlServlet,
> JobPoller, and any other pool-like container class, to add a hook to
> run an AtExit list of hooks. Then, add a utility class that allows
> for singleton per-thread calls, and at-exit calls when the pool
> returns the thread for further processing.
Actually it's very clear!
Jacques
> If this sounds confusing, it's that it's difficult for me to explain,
> and would just be easier if I add the feature(or otherwise show the code).
>
Re: svn commit: r770771 - in /ofbiz/trunk: applications/securityext/src/org/ofbiz/securityext/test/
framework/security/src/org/ofbiz/security/authz/ framework/webapp/src/org/ofbiz/webapp/control/
Posted by Adam Heath <do...@brainfood.com>.
jaz@apache.org wrote:
> Author: jaz
> Date: Fri May 1 17:47:52 2009
> New Revision: 770771
>
> URL: http://svn.apache.org/viewvc?rev=770771&view=rev
> Log:
> Often thread pools do not clear ThreadLocal, implemented a workaround to handle this
Actually, never, until the Thread is shutdown. ThreadLocal is just
for storing stuff against a thread-type key. What you want is a
PoolThreadLocal, which doesn't exist.
I guess I could add code to support the same thing that webslinger
does for this case. It would require modifying ControlServlet,
JobPoller, and any other pool-like container class, to add a hook to
run an AtExit list of hooks. Then, add a utility class that allows
for singleton per-thread calls, and at-exit calls when the pool
returns the thread for further processing.
If this sounds confusing, it's that it's difficult for me to explain,
and would just be easier if I add the feature(or otherwise show the code).