You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Kevin Zhou <zh...@gmail.com> on 2008/12/22 05:02:29 UTC

Using PriviAction instead of PrivilegedAction

I found that most of our codes use a doPriviledged-PrivilegedAction block to
read system properties in Security, LUNI modules.

We already has an implementation of PrivilegedAction as PriviAction which
can get system properties, security properties, security policy and so on.

Shall we modify all the present doPriviledged-PrivilegedAction blocks into a
doPriviledged-PriviAction block? Or can use doPriviledged-PriviAction from
now on?

Re: Using PriviAction instead of PrivilegedAction

Posted by Sean Qiu <se...@gmail.com>.
"find . -name *.java -exec grep  'doPrivileged' {} \; | wc -l"   in
<trunk>/modules/luni/src/main
We get the fact that AccessController.doPrivileged* is called 53 times.
I think it may be worth refacting it.

Is there any comments about Nathan's suggestion code sample?

2008/12/25 Nathan Beyer <nd...@apache.org>

> Agreed. The value is just eliminating some redundant code and if there
> is a lot of calls in a single module, that's why I suggested a util
> class for each module, as appropriate.
>
> -Nathan
>
> On Wed, Dec 24, 2008 at 5:44 AM, Tim Ellison <t....@gmail.com>
> wrote:
> > Pavel Pervov wrote:
> >> As I see it this class only solves the problem of "the more anonymous
> >> classes - the larger disribution is" problem. That is all. I assume we
> >> want to move toward solving that problem, but I'm not quite sure if we
> >> want to achieve it by using _single_ helper class for all the class
> >> library.
> >
> > So how is this use of PriviAction:
> >  new PriviAction<String>("line.separator")
> >
> >
> > producing fewer anonymous classes than this use of PrivilegedAction:
> >
> >  new PrivilegedAction<String>() {
> >    public String run() {
> >      return System.getProperty("line.separator");
> >    }
> >  }
> >
> >
> > It is more compact in source code, but...<shrug/>
> >
> > Regards,
> > Tim
> >
> >
> >> On Wed, Dec 24, 2008 at 2:10 PM, Tim Ellison <t....@gmail.com>
> wrote:
> >>> I didn't see a good answer to Alexei's question...
> >>>
> >>> Alexei Fedotov wrote:
> >>>> Pavel,
> >>>> Kevin didn't mention performance, did he?
> >>>>
> >>>> I believe it is always a trade off between modularity and speed. The
> >>>> performance measurement might be a part of Kevin's arguments if his
> >>>> intentions are to improve the performance. That is why I asked him to
> >>>> elaborate both approaches uncovering intentions.
> >>>>
> >>>> If our internal security interfaces are much quicker on real
> >>>> applications I would wonder why our public interfaces are so slow.
> >>> What problem is the PriviAction solving?  I see the class comment [1]
> says:
> >>>
> >>> "Helper class to avoid multiple anonymous inner class for
> >>> java.security.AccessController#doPrivileged(PrivilegedAction) calls."
> >>>
> >>> Is that really a problem?  For readability or performance?
> >>>
> >>> [1]
> >>>
> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java?revision=486600
> >>>
> >>> Regards,
> >>> Tim
> >>>
> >>
> >
>



-- 
Best Regards
Sean, Xiao Xia Qiu

China Software Development Lab, IBM

Re: Using PriviAction instead of PrivilegedAction

Posted by Nathan Beyer <nd...@apache.org>.
Agreed. The value is just eliminating some redundant code and if there
is a lot of calls in a single module, that's why I suggested a util
class for each module, as appropriate.

-Nathan

On Wed, Dec 24, 2008 at 5:44 AM, Tim Ellison <t....@gmail.com> wrote:
> Pavel Pervov wrote:
>> As I see it this class only solves the problem of "the more anonymous
>> classes - the larger disribution is" problem. That is all. I assume we
>> want to move toward solving that problem, but I'm not quite sure if we
>> want to achieve it by using _single_ helper class for all the class
>> library.
>
> So how is this use of PriviAction:
>  new PriviAction<String>("line.separator")
>
>
> producing fewer anonymous classes than this use of PrivilegedAction:
>
>  new PrivilegedAction<String>() {
>    public String run() {
>      return System.getProperty("line.separator");
>    }
>  }
>
>
> It is more compact in source code, but...<shrug/>
>
> Regards,
> Tim
>
>
>> On Wed, Dec 24, 2008 at 2:10 PM, Tim Ellison <t....@gmail.com> wrote:
>>> I didn't see a good answer to Alexei's question...
>>>
>>> Alexei Fedotov wrote:
>>>> Pavel,
>>>> Kevin didn't mention performance, did he?
>>>>
>>>> I believe it is always a trade off between modularity and speed. The
>>>> performance measurement might be a part of Kevin's arguments if his
>>>> intentions are to improve the performance. That is why I asked him to
>>>> elaborate both approaches uncovering intentions.
>>>>
>>>> If our internal security interfaces are much quicker on real
>>>> applications I would wonder why our public interfaces are so slow.
>>> What problem is the PriviAction solving?  I see the class comment [1] says:
>>>
>>> "Helper class to avoid multiple anonymous inner class for
>>> java.security.AccessController#doPrivileged(PrivilegedAction) calls."
>>>
>>> Is that really a problem?  For readability or performance?
>>>
>>> [1]
>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java?revision=486600
>>>
>>> Regards,
>>> Tim
>>>
>>
>

Re: Using PriviAction instead of PrivilegedAction

Posted by Tim Ellison <t....@gmail.com>.
Pavel Pervov wrote:
> As I see it this class only solves the problem of "the more anonymous
> classes - the larger disribution is" problem. That is all. I assume we
> want to move toward solving that problem, but I'm not quite sure if we
> want to achieve it by using _single_ helper class for all the class
> library.

So how is this use of PriviAction:
  new PriviAction<String>("line.separator")


producing fewer anonymous classes than this use of PrivilegedAction:

  new PrivilegedAction<String>() {
    public String run() {
      return System.getProperty("line.separator");
    }
  }


It is more compact in source code, but...<shrug/>

Regards,
Tim


> On Wed, Dec 24, 2008 at 2:10 PM, Tim Ellison <t....@gmail.com> wrote:
>> I didn't see a good answer to Alexei's question...
>>
>> Alexei Fedotov wrote:
>>> Pavel,
>>> Kevin didn't mention performance, did he?
>>>
>>> I believe it is always a trade off between modularity and speed. The
>>> performance measurement might be a part of Kevin's arguments if his
>>> intentions are to improve the performance. That is why I asked him to
>>> elaborate both approaches uncovering intentions.
>>>
>>> If our internal security interfaces are much quicker on real
>>> applications I would wonder why our public interfaces are so slow.
>> What problem is the PriviAction solving?  I see the class comment [1] says:
>>
>> "Helper class to avoid multiple anonymous inner class for
>> java.security.AccessController#doPrivileged(PrivilegedAction) calls."
>>
>> Is that really a problem?  For readability or performance?
>>
>> [1]
>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java?revision=486600
>>
>> Regards,
>> Tim
>>
> 

Re: Using PriviAction instead of PrivilegedAction

Posted by Pavel Pervov <pm...@gmail.com>.
As I see it this class only solves the problem of "the more anonymous
classes - the larger disribution is" problem. That is all. I assume we
want to move toward solving that problem, but I'm not quite sure if we
want to achieve it by using _single_ helper class for all the class
library.

WBR,
    Pavel.

On Wed, Dec 24, 2008 at 2:10 PM, Tim Ellison <t....@gmail.com> wrote:
> I didn't see a good answer to Alexei's question...
>
> Alexei Fedotov wrote:
>> Pavel,
>> Kevin didn't mention performance, did he?
>>
>> I believe it is always a trade off between modularity and speed. The
>> performance measurement might be a part of Kevin's arguments if his
>> intentions are to improve the performance. That is why I asked him to
>> elaborate both approaches uncovering intentions.
>>
>> If our internal security interfaces are much quicker on real
>> applications I would wonder why our public interfaces are so slow.
>
> What problem is the PriviAction solving?  I see the class comment [1] says:
>
> "Helper class to avoid multiple anonymous inner class for
> java.security.AccessController#doPrivileged(PrivilegedAction) calls."
>
> Is that really a problem?  For readability or performance?
>
> [1]
> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java?revision=486600
>
> Regards,
> Tim
>

Re: Using PriviAction instead of PrivilegedAction

Posted by Tim Ellison <t....@gmail.com>.
I didn't see a good answer to Alexei's question...

Alexei Fedotov wrote:
> Pavel,
> Kevin didn't mention performance, did he?
> 
> I believe it is always a trade off between modularity and speed. The
> performance measurement might be a part of Kevin's arguments if his
> intentions are to improve the performance. That is why I asked him to
> elaborate both approaches uncovering intentions.
> 
> If our internal security interfaces are much quicker on real
> applications I would wonder why our public interfaces are so slow.

What problem is the PriviAction solving?  I see the class comment [1] says:

"Helper class to avoid multiple anonymous inner class for
java.security.AccessController#doPrivileged(PrivilegedAction) calls."

Is that really a problem?  For readability or performance?

[1]
http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java?revision=486600

Regards,
Tim

Re: Using PriviAction instead of PrivilegedAction

Posted by Regis <xu...@gmail.com>.

Alexei Fedotov wrote:
> Well, I would say it was pretty well formatted.
> 
> The thing I don't like about this class is a mess of different
> security applications which cannot be deducted from naming. The class
> would be easier to understand if split into four appropriately named
> actions. This would also help renaming arg1, arg2 fields into
> something readable.
I raise a JIRA[1], and attach a simple patch to rename field arg1 to 
target and arg2 to defaultAnswer as my understanding. I also add four 
factory methods to help create PriviAction instance. I think there are 
too many constructors that are confused, so the next step is hide them 
by marking to private. What do you think? Any suggestions/comments?
> 
> 
> 
> On Tue, Dec 23, 2008 at 12:42 PM, Kevin Zhou <zh...@gmail.com> wrote:
>> Is this PriviAction really an ugly class?
>>
> 
> 
> 

-- 
Best Regards,
Regis.

Re: Using PriviAction instead of PrivilegedAction

Posted by Regis <xu...@gmail.com>.

Kevin Zhou wrote:
> Regis's JIRA [1]
> 
> [1] https://issues.apache.org/jira/browse/HARMONY-6060
> 
Thanks Kevin, I forgot to paste the URL :)

-- 
Best Regards,
Regis.

Re: Using PriviAction instead of PrivilegedAction

Posted by Kevin Zhou <zh...@gmail.com>.
Regis's JIRA [1]

[1] https://issues.apache.org/jira/browse/HARMONY-6060

Re: Using PriviAction instead of PrivilegedAction

Posted by Nathan Beyer <nd...@apache.org>.
I was hacking around the other day and was thinking about a single
util class (below). In this case, I implemented a 'get system
property' and a 'get system property, load class name for value,
construct it'.

This would then be used like this.

PrivilegedActions.getSystemProperty(pn, null);

or with a static import

getSystemProperty(pn, null);

-- quick util class for privileged actions --

public final class PrivilegedActions {

    private static class GetSystemPropertyAction implements
PrivilegedAction<String> {
        private final String propertyName;
        private final String defaultValue;

        private GetSystemPropertyAction(String propertyName, String
defaultValue) {
            super();
            assert propertyName != null && propertyName.length() > 0;
            this.propertyName = propertyName;
            this.defaultValue = defaultValue;
        }

        public String run() {
            return System.getProperty(propertyName, defaultValue);
        }
    }

    private static final class ConstructClassFromSystemProperty<T>
implements PrivilegedAction<T> {
        private final String propertyName;

        private ConstructClassFromSystemProperty(String propertyName) {
            super();
            assert propertyName != null && propertyName.length() > 0;
            this.propertyName = propertyName;
        }

        public T run() {
            final String className = System.getProperty(propertyName);
            if (className == null) {
                return null;
            }
            ClassLoader cl = Thread.currentThread().getContextClassLoader();
            if (cl == null) {
                cl = ClassLoader.getSystemClassLoader();
            }
            try {
                @SuppressWarnings("unchecked")
                final Class<T> clazz = (Class<T>)
Class.forName(className, true, cl);
                return clazz.newInstance();
            } catch (Exception e) {
                return null;
            }
        }
    }

    public static String getSystemProperty(String propertyName, String
defaultValue) {
        return doPrivileged(new GetSystemPropertyAction(propertyName,
defaultValue));
    }

    public static <T> T constructClassFromSystemProperty(String propertyName) {
        return doPrivileged(new
ConstructClassFromSystemProperty<T>(propertyName));
    }

    private PrivilegedActions() {
    }
}

On Wed, Dec 24, 2008 at 1:12 AM, Alexei Fedotov
<al...@gmail.com> wrote:
> Nathan,
>
> Do you think that splitting the PriviAction class into four classes
> (GetSystemPropertyPriviAction, GetSecurityPolicyPriviAction,
> SetAccessiblePriviAction, GetSecurityPropertyPriviAction) would help
> clearing the mess of the initial combined class?
>
> Thanks!
>
>
> On Wed, Dec 24, 2008 at 2:46 AM, Nathan Beyer <nd...@apache.org> wrote:
>> I meant that it's a logical mess - it does a number of different
>> things and it's not what I would call elegant code.
>>
>> If a more central and common utility is desired, then I'd suggest
>> creating one from scratch to replace PriviAction.
>>
>> -Nathan
>>
>> On Tue, Dec 23, 2008 at 3:51 AM, Alexei Fedotov
>> <al...@gmail.com> wrote:
>>> Well, I would say it was pretty well formatted.
>>>
>>> The thing I don't like about this class is a mess of different
>>> security applications which cannot be deducted from naming. The class
>>> would be easier to understand if split into four appropriately named
>>> actions. This would also help renaming arg1, arg2 fields into
>>> something readable.
>>>
>>>
>>>
>>> On Tue, Dec 23, 2008 at 12:42 PM, Kevin Zhou <zh...@gmail.com> wrote:
>>>> Is this PriviAction really an ugly class?
>>>>
>>>
>>>
>>>
>>> --
>>> С уважением,
>>> Алексей Федотов,
>>> ЗАО «Телеком Экспресс»
>>>
>>
>
>
>
> --
> С уважением,
> Алексей Федотов,
> ЗАО «Телеком Экспресс»
>

Re: Using PriviAction instead of PrivilegedAction

Posted by Alexei Fedotov <al...@gmail.com>.
Nathan,

Do you think that splitting the PriviAction class into four classes
(GetSystemPropertyPriviAction, GetSecurityPolicyPriviAction,
SetAccessiblePriviAction, GetSecurityPropertyPriviAction) would help
clearing the mess of the initial combined class?

Thanks!


On Wed, Dec 24, 2008 at 2:46 AM, Nathan Beyer <nd...@apache.org> wrote:
> I meant that it's a logical mess - it does a number of different
> things and it's not what I would call elegant code.
>
> If a more central and common utility is desired, then I'd suggest
> creating one from scratch to replace PriviAction.
>
> -Nathan
>
> On Tue, Dec 23, 2008 at 3:51 AM, Alexei Fedotov
> <al...@gmail.com> wrote:
>> Well, I would say it was pretty well formatted.
>>
>> The thing I don't like about this class is a mess of different
>> security applications which cannot be deducted from naming. The class
>> would be easier to understand if split into four appropriately named
>> actions. This would also help renaming arg1, arg2 fields into
>> something readable.
>>
>>
>>
>> On Tue, Dec 23, 2008 at 12:42 PM, Kevin Zhou <zh...@gmail.com> wrote:
>>> Is this PriviAction really an ugly class?
>>>
>>
>>
>>
>> --
>> С уважением,
>> Алексей Федотов,
>> ЗАО «Телеком Экспресс»
>>
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: Using PriviAction instead of PrivilegedAction

Posted by Nathan Beyer <nd...@apache.org>.
I meant that it's a logical mess - it does a number of different
things and it's not what I would call elegant code.

If a more central and common utility is desired, then I'd suggest
creating one from scratch to replace PriviAction.

-Nathan

On Tue, Dec 23, 2008 at 3:51 AM, Alexei Fedotov
<al...@gmail.com> wrote:
> Well, I would say it was pretty well formatted.
>
> The thing I don't like about this class is a mess of different
> security applications which cannot be deducted from naming. The class
> would be easier to understand if split into four appropriately named
> actions. This would also help renaming arg1, arg2 fields into
> something readable.
>
>
>
> On Tue, Dec 23, 2008 at 12:42 PM, Kevin Zhou <zh...@gmail.com> wrote:
>> Is this PriviAction really an ugly class?
>>
>
>
>
> --
> С уважением,
> Алексей Федотов,
> ЗАО «Телеком Экспресс»
>

Re: Using PriviAction instead of PrivilegedAction

Posted by Alexei Fedotov <al...@gmail.com>.
Well, I would say it was pretty well formatted.

The thing I don't like about this class is a mess of different
security applications which cannot be deducted from naming. The class
would be easier to understand if split into four appropriately named
actions. This would also help renaming arg1, arg2 fields into
something readable.



On Tue, Dec 23, 2008 at 12:42 PM, Kevin Zhou <zh...@gmail.com> wrote:
> Is this PriviAction really an ugly class?
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: Using PriviAction instead of PrivilegedAction

Posted by Kevin Zhou <zh...@gmail.com>.
Is this PriviAction really an ugly class?

Re: Using PriviAction instead of PrivilegedAction

Posted by Regis <xu...@gmail.com>.

Alexei Fedotov wrote:
>> every module already depends on luni module
> 
> I found luni dependencies in the following modules:archive auth, awt,
> imageio, nio, prefs, security, sql, suncompat, text, x-net. Others
> does not seem to depend from luni. Looking deeper into these
The dependency I mean include both public API and internal API, so no 
one could escape from luni ;) , and I agree with you that the dependent 
of internal API should be removed first.
> dependencies I convinced myself that it might be a good idea to
> separate generic java helpers which resided in
> org.apache.harmony.luni.util into a separate helper module. I believe
+1
Does "misc" module do for this?
> this should not apply to the PriviAction: this secure utility should
> be moved to the security module instead to keep things properly
> packed.
This just move the dependency from luni to security module, if we still 
use PriviAction. I didn't look implementation of PriviAction deeply, but 
if it was mess as Nathan said, we can clean it up first :)

> 
> Most packages use java.lang.* but not necessarily internal
> undocumented helpers. This allows plugging our library modules into
> different VMs using standard interface.
> 
> Thanks.
> 
> 
> On Tue, Dec 23, 2008 at 8:36 AM, Regis <xu...@gmail.com> wrote:
>>
>> Nathan Beyer wrote:
>>> On Mon, Dec 22, 2008 at 11:15 PM, Kevin Zhou <zh...@gmail.com>
>>> wrote:
>>>> Nathan, thanks for your comments.
>>>>
>>>> Nathan wrote,
>>>>> This would create a dependency on every module to an implementation
>>>> package in LUNI.
>>>> Yes, that would be.
>>>>
>>>>> I'd suggest each module create one helper class, as needed, and reuse
>>>>> that
>>>> internally.
>>>> Adding an internally-used helper class can improve modularity. But I
>>>> doubt
>>>> that it may bring redundant codes.
>>>> For example, both LUNI and Security needs to access the system
>>>> properties.
>>>> In fact, PriviAction implements PrivilegedAction and only provides
>>>> services
>>>> to get system properties, security properties and so.
>>> I understand, but eliminating redundant code isn't an absolute value
>>> -- it's not always a good thing. Reuse must be balanced with the
>>> requirements of modularity and the needs of isolation.
>> Yes, it's trade off, but seems every module already depends on luni module,
>> another helper class may could not help more.
>>
>>> -Nathan
>>>
>>>>> As I recall, PriviAction is quite an ugly class - it's rather a mess.
>>>> What about improving this helper class instead of adding an internal
>>>> helper
>>>> classes in each module?
>>>>
>> --
>> Best Regards,
>> Regis.
>>
> 
> 
> 

-- 
Best Regards,
Regis.

Re: Using PriviAction instead of PrivilegedAction

Posted by Alexei Fedotov <al...@gmail.com>.
> every module already depends on luni module

I found luni dependencies in the following modules:archive auth, awt,
imageio, nio, prefs, security, sql, suncompat, text, x-net. Others
does not seem to depend from luni. Looking deeper into these
dependencies I convinced myself that it might be a good idea to
separate generic java helpers which resided in
org.apache.harmony.luni.util into a separate helper module. I believe
this should not apply to the PriviAction: this secure utility should
be moved to the security module instead to keep things properly
packed.

Most packages use java.lang.* but not necessarily internal
undocumented helpers. This allows plugging our library modules into
different VMs using standard interface.

Thanks.


On Tue, Dec 23, 2008 at 8:36 AM, Regis <xu...@gmail.com> wrote:
>
>
> Nathan Beyer wrote:
>>
>> On Mon, Dec 22, 2008 at 11:15 PM, Kevin Zhou <zh...@gmail.com>
>> wrote:
>>>
>>> Nathan, thanks for your comments.
>>>
>>> Nathan wrote,
>>>>
>>>> This would create a dependency on every module to an implementation
>>>
>>> package in LUNI.
>>> Yes, that would be.
>>>
>>>> I'd suggest each module create one helper class, as needed, and reuse
>>>> that
>>>
>>> internally.
>>> Adding an internally-used helper class can improve modularity. But I
>>> doubt
>>> that it may bring redundant codes.
>>> For example, both LUNI and Security needs to access the system
>>> properties.
>>> In fact, PriviAction implements PrivilegedAction and only provides
>>> services
>>> to get system properties, security properties and so.
>>
>> I understand, but eliminating redundant code isn't an absolute value
>> -- it's not always a good thing. Reuse must be balanced with the
>> requirements of modularity and the needs of isolation.
>
> Yes, it's trade off, but seems every module already depends on luni module,
> another helper class may could not help more.
>
>>
>> -Nathan
>>
>>>> As I recall, PriviAction is quite an ugly class - it's rather a mess.
>>>
>>> What about improving this helper class instead of adding an internal
>>> helper
>>> classes in each module?
>>>
>>
>
> --
> Best Regards,
> Regis.
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: Using PriviAction instead of PrivilegedAction

Posted by Regis <xu...@gmail.com>.

Nathan Beyer wrote:
> On Mon, Dec 22, 2008 at 11:15 PM, Kevin Zhou <zh...@gmail.com> wrote:
>> Nathan, thanks for your comments.
>>
>> Nathan wrote,
>>> This would create a dependency on every module to an implementation
>> package in LUNI.
>> Yes, that would be.
>>
>>> I'd suggest each module create one helper class, as needed, and reuse that
>> internally.
>> Adding an internally-used helper class can improve modularity. But I doubt
>> that it may bring redundant codes.
>> For example, both LUNI and Security needs to access the system properties.
>> In fact, PriviAction implements PrivilegedAction and only provides services
>> to get system properties, security properties and so.
> 
> I understand, but eliminating redundant code isn't an absolute value
> -- it's not always a good thing. Reuse must be balanced with the
> requirements of modularity and the needs of isolation.

Yes, it's trade off, but seems every module already depends on luni 
module, another helper class may could not help more.

> 
> -Nathan
> 
>>> As I recall, PriviAction is quite an ugly class - it's rather a mess.
>> What about improving this helper class instead of adding an internal helper
>> classes in each module?
>>
> 

-- 
Best Regards,
Regis.

Re: Using PriviAction instead of PrivilegedAction

Posted by Nathan Beyer <nd...@apache.org>.
On Mon, Dec 22, 2008 at 11:15 PM, Kevin Zhou <zh...@gmail.com> wrote:
> Nathan, thanks for your comments.
>
> Nathan wrote,
>> This would create a dependency on every module to an implementation
> package in LUNI.
> Yes, that would be.
>
>> I'd suggest each module create one helper class, as needed, and reuse that
> internally.
> Adding an internally-used helper class can improve modularity. But I doubt
> that it may bring redundant codes.
> For example, both LUNI and Security needs to access the system properties.
> In fact, PriviAction implements PrivilegedAction and only provides services
> to get system properties, security properties and so.

I understand, but eliminating redundant code isn't an absolute value
-- it's not always a good thing. Reuse must be balanced with the
requirements of modularity and the needs of isolation.

-Nathan

>
>> As I recall, PriviAction is quite an ugly class - it's rather a mess.
> What about improving this helper class instead of adding an internal helper
> classes in each module?
>

Re: Using PriviAction instead of PrivilegedAction

Posted by Kevin Zhou <zh...@gmail.com>.
Nathan, thanks for your comments.

Nathan wrote,
> This would create a dependency on every module to an implementation
package in LUNI.
Yes, that would be.

> I'd suggest each module create one helper class, as needed, and reuse that
internally.
Adding an internally-used helper class can improve modularity. But I doubt
that it may bring redundant codes.
For example, both LUNI and Security needs to access the system properties.
In fact, PriviAction implements PrivilegedAction and only provides services
to get system properties, security properties and so.

> As I recall, PriviAction is quite an ugly class - it's rather a mess.
What about improving this helper class instead of adding an internal helper
classes in each module?

Re: Using PriviAction instead of PrivilegedAction

Posted by Nathan Beyer <nd...@apache.org>.
On Mon, Dec 22, 2008 at 8:18 PM, Kevin Zhou <zh...@gmail.com> wrote:
> Alexei wrote,
>> Could you please elaborate both variants a bit?
> Yes. PrivilegedAction is an interface which is used only for computations
> that do not throw checked exceptions. We also has a PriviAction
> implementation of PrivilegedAction interface. PriviAction is a helper class
> to avoid multiple anonymous inner class for doPriviledged/PrivilegedAction
> calls.
>
>> It is always a trade off between modularity and speed.
> :) Every time when you implement a doPriviledged/PrivilegedAction block, you
> need to add a anonymous inner class. PriviAction wraps some common
> doPriviledged/PrivilegedAction calls for system properties, security
> properties, security policy and so on.
>
> In a word, PriviAction can help to avoid multiple anonymous inner classes to
> make our code look better.
>
>> The performance measurement might be a part of Kevin's arguments.
> In fact, I don't know whether PriviAction may improve any performance.
> Only since we have such a PriviAction helper class, why not use it instead?
>

Modularity would be the reason. This would create a dependency on
every module to an implementation package in LUNI. As an alternative,
I'd suggest each module create one helper class, as needed, and reuse
that internally.

As I recall, PriviAction is quite an ugly class - it's rather a mess.

-Nathan

Re: Using PriviAction instead of PrivilegedAction

Posted by Kevin Zhou <zh...@gmail.com>.
Alexei wrote,
> Could you please elaborate both variants a bit?
Yes. PrivilegedAction is an interface which is used only for computations
that do not throw checked exceptions. We also has a PriviAction
implementation of PrivilegedAction interface. PriviAction is a helper class
to avoid multiple anonymous inner class for doPriviledged/PrivilegedAction
calls.

> It is always a trade off between modularity and speed.
:) Every time when you implement a doPriviledged/PrivilegedAction block, you
need to add a anonymous inner class. PriviAction wraps some common
doPriviledged/PrivilegedAction calls for system properties, security
properties, security policy and so on.

In a word, PriviAction can help to avoid multiple anonymous inner classes to
make our code look better.

> The performance measurement might be a part of Kevin's arguments.
In fact, I don't know whether PriviAction may improve any performance.
Only since we have such a PriviAction helper class, why not use it instead?

Re: Using PriviAction instead of PrivilegedAction

Posted by Alexei Fedotov <al...@gmail.com>.
Pavel,
Kevin didn't mention performance, did he?

I believe it is always a trade off between modularity and speed. The
performance measurement might be a part of Kevin's arguments if his
intentions are to improve the performance. That is why I asked him to
elaborate both approaches uncovering intentions.

If our internal security interfaces are much quicker on real
applications I would wonder why our public interfaces are so slow.
Thanks.

On Mon, Dec 22, 2008 at 1:40 PM, Pavel Pervov <pm...@gmail.com> wrote:
> Hm... From my POV using implementation specific classes which can
> improve performance is preffered even in implementation of public
> interfaces. This is what gives advantage of one implementation over
> the other.
>
> WBR,
>   Pavel.
>
> On Mon, Dec 22, 2008 at 12:32 PM, Alexei Fedotov
> <al...@gmail.com> wrote:
>> Kevin,
>>
>> Could you please elaborate both variants a bit? I would blindly say
>> that using public documented interfaces
>> (doPriviledged/PrivilegedAction) from most modules might be good idea
>> even if they were slower.
>>
>> Thanks!
>>
>> On Mon, Dec 22, 2008 at 8:52 AM, Kevin Zhou <zh...@gmail.com> wrote:
>>> +
>>>
>>
>>
>>
>> --
>> С уважением,
>> Алексей Федотов,
>> ЗАО «Телеком Экспресс»
>>
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: Using PriviAction instead of PrivilegedAction

Posted by Pavel Pervov <pm...@gmail.com>.
Hm... From my POV using implementation specific classes which can
improve performance is preffered even in implementation of public
interfaces. This is what gives advantage of one implementation over
the other.

WBR,
   Pavel.

On Mon, Dec 22, 2008 at 12:32 PM, Alexei Fedotov
<al...@gmail.com> wrote:
> Kevin,
>
> Could you please elaborate both variants a bit? I would blindly say
> that using public documented interfaces
> (doPriviledged/PrivilegedAction) from most modules might be good idea
> even if they were slower.
>
> Thanks!
>
> On Mon, Dec 22, 2008 at 8:52 AM, Kevin Zhou <zh...@gmail.com> wrote:
>> +
>>
>
>
>
> --
> С уважением,
> Алексей Федотов,
> ЗАО «Телеком Экспресс»
>

Re: Using PriviAction instead of PrivilegedAction

Posted by Alexei Fedotov <al...@gmail.com>.
Kevin,

Could you please elaborate both variants a bit? I would blindly say
that using public documented interfaces
(doPriviledged/PrivilegedAction) from most modules might be good idea
even if they were slower.

Thanks!

On Mon, Dec 22, 2008 at 8:52 AM, Kevin Zhou <zh...@gmail.com> wrote:
> +
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: Using PriviAction instead of PrivilegedAction

Posted by Kevin Zhou <zh...@gmail.com>.
+