Jonathan Costers wrote:
Hi Peter
You are talking about this change, correct?
Yes.
/**
* Permission required to dynamically grant permissions by security policy
@@ -554,7 +555,7 @@ public final class GrantPermission exten
* of permissions.
*/
private static String constructName(Permission[] pa) {
- StringBuffer sb = new StringBuffer(60);
+ StringBuffer sb = new StringBuffer();
16 characters wasn't enough to avoid resizing.
-> I'm curious, why is the above?
for (int i = 0; i < pa.length; i++) {
Permission p = pa[i];
if (p instanceof UnresolvedPermission) {
@@ -675,7 +676,7 @@ public final class GrantPermission exten
private static class Implier {
private final PermissionCollection perms = new Permissions();
- private final List unresolved =new ArrayList();
+ private final ArrayList unresolved = new ArrayList();
-> I'm curious, why is the above?
Can't remember, there shouldn't be too many unresolved permissions, so
I've haven't set a default size greater than 16, I've probably changed
the ArrayList to List, simply because List is a standard interface and
no ArrayList specific methods were used. Minor Refactoring.
void add(GrantPermission gp) {
for (int i = 0; i < gp.grants.length; i++) {
@@ -752,8 +753,7 @@ public final class GrantPermission exten
* @serial include
*/
static class GrantPermissionCollection extends PermissionCollection {
- // All access is synchronized through GrantPermissionCollection
- // Nothing within should use synchronization
+
private static final long serialVersionUID = 8227621799817733985L;
/**
@@ -763,7 +763,7 @@ public final class GrantPermission exten
new ObjectStreamField("perms", List.class, true)
};
- private List perms = new ArrayList(40);
+ private List perms = new ArrayList();
-> I'm curious, why is the above?
private Implier implier = new Implier();
public synchronized void add(Permission p) {
@@ -774,10 +774,8 @@ public final class GrantPermission exten
throw new SecurityException(
"can't add to read-only PermissionCollection");
}
- if (!perms.contains(p)){
- perms.add(p);
- implier.add((GrantPermission) p);
- }
+ perms.add(p);
+ implier.add((GrantPermission) p);
}
-> I'm curious, why is the above?
This prevents duplicates being added to perms, under some circumstances
a Memory exception occurred while expanding UmbrellaGrantPermission
dynamic grants, in the DynamicPolicy (ConcurrentDynamicPolicyProvider),
DynamicPolicyProvider doesn't expand UmbrellaGrantPermission's. Feature
was requested on Jira.
I'd have to go over the code again to remember exactly how it was
caused, but this was part of the fix, it was something to do with
expanding UmbrellaGrant's getting caught in an ever expanding loop.
I added a comment about synchronization also.
Peter.
Sorry again, I was quite tired last night ... This slipped my attention
while doing all these tiny changes to fix javadoc.
Best
Jonathan
2010/9/21 Jonathan Costers <[email protected]>
I will restore your improvements, apologies.
What I wanted to do is back out some of your import removals, since they
broke javadoc generation for those classes.
Good to see people are awake :-)
2010/9/21 Peter Firmstone <[email protected]>
I'm curious about these changes, in your commit, what concerns me here is
not the removal of some performance improvements, but a bug that I found
while expanding UmbrellaGrant's.
I guess I should have created a regression test and documented it.
It relates to a memory out of bounds exception that is thrown if a dynamic
policy supports the use of UmbrellaGrantPermission, as requested on JIRA.
Not everything I did should be considered experimental. I've removed all
experimental changes.
Rather than remove my hard work, wouldn't it be better to just find the
build that passes all tests in svn, like Patricia was doing?
I'm assuming you removed it so a test passes? If this change caused a
test failure, then the bug is not this piece of code, we should be
investigating why this change causes that test to fail, so we can solve the
bug, rather than obscure it.
Peter.
[email protected] wrote:
Modified:
incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
URL:
http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java?rev=999115&r1=999114&r2=999115&view=diff
==============================================================================
--- incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
(original)
+++ incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
Mon Sep 20 20:56:19 2010
@@ -41,6 +41,7 @@ import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import net.jini.security.policy.DynamicPolicy;
/**
* Permission required to dynamically grant permissions by security
policy
@@ -554,7 +555,7 @@ public final class GrantPermission exten
* of permissions.
*/
private static String constructName(Permission[] pa) {
- StringBuffer sb = new StringBuffer(60);
+ StringBuffer sb = new StringBuffer();
for (int i = 0; i < pa.length; i++) {
Permission p = pa[i];
if (p instanceof UnresolvedPermission) {
@@ -675,7 +676,7 @@ public final class GrantPermission exten
private static class Implier {
private final PermissionCollection perms = new Permissions();
- private final List unresolved =new ArrayList();
+ private final ArrayList unresolved = new ArrayList();
void add(GrantPermission gp) {
for (int i = 0; i < gp.grants.length; i++) {
@@ -752,8 +753,7 @@ public final class GrantPermission exten
* @serial include
*/
static class GrantPermissionCollection extends PermissionCollection {
- // All access is synchronized through GrantPermissionCollection
- // Nothing within should use synchronization +
private static final long serialVersionUID = 8227621799817733985L;
/**
@@ -763,7 +763,7 @@ public final class GrantPermission exten
new ObjectStreamField("perms", List.class, true)
};
- private List perms = new ArrayList(40);
+ private List perms = new ArrayList();
private Implier implier = new Implier();
public synchronized void add(Permission p) {
@@ -774,10 +774,8 @@ public final class GrantPermission exten
throw new SecurityException(
"can't add to read-only PermissionCollection");
}
- if (!perms.contains(p)){
- perms.add(p);
- implier.add((GrantPermission) p);
- }
+ perms.add(p);
+ implier.add((GrantPermission) p);
}
public synchronized Enumeration elements() {
