You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fortress@directory.apache.org by Chris Pike <cl...@psu.edu> on 2016/03/12 02:01:53 UTC

Static Config Initialization Problems

We need to handle the config initialization differently. See the static init block in this class...

https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/util/Config.java

If this fails when the application starts, it throws an exception and is never reinitialized. This results in all further requests throwing NoClassDefFoundError (http://stackoverflow.com/questions/6352215/java-why-java-lang-noclassdeffounderror-caused-by-static-field-initializ-failur)

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
Yeah, looks like running from the command line is causing infinite loop. Is the command line "mvn -Dtest=FortressJUnitTest test" still using ant?



----- Original Message -----
From: "Chris Pike" <cl...@psu.edu>
To: fortress@directory.apache.org
Sent: Tuesday, April 26, 2016 4:16:15 PM
Subject: Re: Static Config Initialization Problems

No, I wouldn't expect that, I've been running the tests from eclipse and don't see that happening. I'll try running from command line and see what happens.


----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, April 26, 2016 4:02:34 PM
Subject: Re: Static Config Initialization Problems

> On Apr 26, 2016, at 2:26 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> 
> I went ahead and refactored to not use static init blocks. There was a huge ripple effect through the code and I ran into some other problems along the way. (had to separate out local config from remote config and make some changes to the DAOs). 
> 
> https://github.com/PennState/directory-fortress-core-1/commits/feature/modifyBootstrapSingleton
> 
> Let me know what you think.


Built and running junit tests.  Haven’t had time to walk the initialization with a debugger but it appears to be iterating over the same code sequences. Is this expected?  It’s generating a lot of INFO messages.

2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
...

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
No, I wouldn't expect that, I've been running the tests from eclipse and don't see that happening. I'll try running from command line and see what happens.


----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, April 26, 2016 4:02:34 PM
Subject: Re: Static Config Initialization Problems

> On Apr 26, 2016, at 2:26 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> 
> I went ahead and refactored to not use static init blocks. There was a huge ripple effect through the code and I ran into some other problems along the way. (had to separate out local config from remote config and make some changes to the DAOs). 
> 
> https://github.com/PennState/directory-fortress-core-1/commits/feature/modifyBootstrapSingleton
> 
> Let me know what you think.


Built and running junit tests.  Haven’t had time to walk the initialization with a debugger but it appears to be iterating over the same code sequences. Is this expected?  It’s generating a lot of INFO messages.

2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
...

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 26, 2016, at 2:26 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> 
> I went ahead and refactored to not use static init blocks. There was a huge ripple effect through the code and I ran into some other problems along the way. (had to separate out local config from remote config and make some changes to the DAOs). 
> 
> https://github.com/PennState/directory-fortress-core-1/commits/feature/modifyBootstrapSingleton
> 
> Let me know what you think.


Built and running junit tests.  Haven’t had time to walk the initialization with a debugger but it appears to be iterating over the same code sequences. Is this expected?  It’s generating a lot of INFO messages.

2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  Config:80 - static init: load config realm [DEFAULT]
2016-04-24 18:15:045 INFO  ConfigDAO:268 - getConfig dn [cn=DEFAULT,ou=Config,dc=example,dc=com]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
2016-04-24 18:15:045 INFO  LdapConnectionProvider:96 - LDAP POOL:  host=[localhost], port=[389], min=[1], max=[10]
...

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
Shawn,

I went ahead and refactored to not use static init blocks. There was a huge ripple effect through the code and I ran into some other problems along the way. (had to separate out local config from remote config and make some changes to the DAOs). 

https://github.com/PennState/directory-fortress-core-1/commits/feature/modifyBootstrapSingleton

Let me know what you think.

~Chris


----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, April 19, 2016 11:28:53 AM
Subject: Re: Static Config Initialization Problems

> On Apr 19, 2016, at 10:18 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> We can't use a static variable since it would result in the same problem we are trying to fix (exceptions on static initialization)

ah crap you’re right.

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 19, 2016, at 10:18 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> We can't use a static variable since it would result in the same problem we are trying to fix (exceptions on static initialization)

ah crap you’re right.

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
We can't use a static variable since it would result in the same problem we are trying to fix (exceptions on static initialization)



----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, April 19, 2016 9:59:44 AM
Subject: Re: Static Config Initialization Problems

> On Apr 19, 2016, at 7:34 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Ok, I'll work on my branch and get the tests working. What do you mean by wrap Config.getInstance() with CONFIG?

Cool.  

Please use a method wrapper such as this for the code references:

public final class GlobalIds
{

...
    public static final Config CONFIG = Config.getInstance(  );

or if you have a better way that’s fine too.  Don’t want to see Config.getIntance().getProperty(…) in a kazillion places.

Thanks,

Shawn

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 19, 2016, at 7:34 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Ok, I'll work on my branch and get the tests working. What do you mean by wrap Config.getInstance() with CONFIG?

Cool.  

Please use a method wrapper such as this for the code references:

public final class GlobalIds
{

...
    public static final Config CONFIG = Config.getInstance(  );

or if you have a better way that’s fine too.  Don’t want to see Config.getIntance().getProperty(…) in a kazillion places.

Thanks,

Shawn

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
Ok, I'll work on my branch and get the tests working. What do you mean by wrap Config.getInstance() with CONFIG?



----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Monday, April 18, 2016 8:49:06 AM
Subject: Re: Static Config Initialization Problems

> On Apr 18, 2016, at 7:07 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Here is the how the config class would be changed.
> 
> https://github.com/PennState/directory-fortress-core-1/blob/feature/modifyBootstrapSingleton/src/main/java/org/apache/directory/fortress/core/util/Config.java#L75

Looks simple enough.  

Before you said:
"In most cases, I think we can simply remove the static variable and put a method call to Confg.getInstance().getProperty into the method. In the case of other static blocks like ApacheDsDataProvider, they will probably have to be modified to have the same new singleton pattern as Config. There is a ripple effect through the code if we take this approach, so wanted to get everyone's buy in before I make all the changes."

The ripple effect is what concerns me.  I’d say apply the change down in your branch, get the junit tests working, and then we'll know.  To keep the code readable, wrap Config.getInstance() with CONFIG or some such thing because that reference will be in about 140 locations.

Shawn

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 18, 2016, at 7:07 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Here is the how the config class would be changed.
> 
> https://github.com/PennState/directory-fortress-core-1/blob/feature/modifyBootstrapSingleton/src/main/java/org/apache/directory/fortress/core/util/Config.java#L75

Looks simple enough.  

Before you said:
"In most cases, I think we can simply remove the static variable and put a method call to Confg.getInstance().getProperty into the method. In the case of other static blocks like ApacheDsDataProvider, they will probably have to be modified to have the same new singleton pattern as Config. There is a ripple effect through the code if we take this approach, so wanted to get everyone's buy in before I make all the changes."

The ripple effect is what concerns me.  I’d say apply the change down in your branch, get the junit tests working, and then we'll know.  To keep the code readable, wrap Config.getInstance() with CONFIG or some such thing because that reference will be in about 140 locations.

Shawn



Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
Here is the how the config class would be changed.

https://github.com/PennState/directory-fortress-core-1/blob/feature/modifyBootstrapSingleton/src/main/java/org/apache/directory/fortress/core/util/Config.java#L75




----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Sunday, April 17, 2016 10:21:37 PM
Subject: Re: Static Config Initialization Problems

> On Apr 17, 2016, at 5:46 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> I'd like to make these changes soon, but want to make sure you are still OK with the proposed changes before I start.

Chris where is this code so I can have a look?

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 17, 2016, at 5:46 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> I'd like to make these changes soon, but want to make sure you are still OK with the proposed changes before I start.

Chris where is this code so I can have a look?

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
Shawn,

I'd like to make these changes soon, but want to make sure you are still OK with the proposed changes before I start.

~Chris P.


----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Wednesday, March 16, 2016 12:32:41 PM
Subject: Re: Static Config Initialization Problems

> On Mar 16, 2016, at 10:52 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> 1. Don't need to code retry logic. If getInstance() throws exception, INSTANCE will still be null so next call will just try again.

OK

> 
> On Mar 16, 2016, at 10:52 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> 2/3. This will depend on the situation. In most cases, I think we can simply remove the static variable and put a method call to Confg.getInstance().getProperty into the method. In the case of other static blocks like ApacheDsDataProvider, they will probably have to be modified to have the same new singleton pattern as Config. There is a ripple effect through the code if we take this approach, so wanted to get everyone's buy in before I make all the changes.
> 

yes that was my point.  Sounds like you know that already…. :-)

> 
> On Mar 16, 2016, at 10:52 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Side Note: may want to change ApacheDsDataProvider class name since it's not specific to ApacheDS, correct?

Correct, it is specific to apache ldap api, not the directory.  Maybe LdapDataProvider would be more appropriate name.

Shawn

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 16, 2016, at 10:52 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> 1. Don't need to code retry logic. If getInstance() throws exception, INSTANCE will still be null so next call will just try again.

OK

> 
> On Mar 16, 2016, at 10:52 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> 2/3. This will depend on the situation. In most cases, I think we can simply remove the static variable and put a method call to Confg.getInstance().getProperty into the method. In the case of other static blocks like ApacheDsDataProvider, they will probably have to be modified to have the same new singleton pattern as Config. There is a ripple effect through the code if we take this approach, so wanted to get everyone's buy in before I make all the changes.
> 

yes that was my point.  Sounds like you know that already…. :-)

> 
> On Mar 16, 2016, at 10:52 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Side Note: may want to change ApacheDsDataProvider class name since it's not specific to ApacheDS, correct?

Correct, it is specific to apache ldap api, not the directory.  Maybe LdapDataProvider would be more appropriate name.

Shawn

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
1. Don't need to code retry logic. If getInstance() throws exception, INSTANCE will still be null so next call will just try again.

2/3. This will depend on the situation. In most cases, I think we can simply remove the static variable and put a method call to Confg.getInstance().getProperty into the method. In the case of other static blocks like ApacheDsDataProvider, they will probably have to be modified to have the same new singleton pattern as Config. There is a ripple effect through the code if we take this approach, so wanted to get everyone's buy in before I make all the changes.

Side Note: may want to change ApacheDsDataProvider class name since it's not specific to ApacheDS, correct?



----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Wednesday, March 16, 2016 10:49:51 AM
Subject: Re: Static Config Initialization Problems

> On Mar 16, 2016, at 9:44 AM, Shawn McKinney <sm...@apache.org> wrote:
> 
> 2. The config object is used throughout the code, in static blocks, static variable initializations and such.

Left question off #2, but is basically same as #3.  How would you recode the classes using the config instance in static variables or blocks?

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 16, 2016, at 9:44 AM, Shawn McKinney <sm...@apache.org> wrote:
> 
> 2. The config object is used throughout the code, in static blocks, static variable initializations and such.

Left question off #2, but is basically same as #3.  How would you recode the classes using the config instance in static variables or blocks?

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 16, 2016, at 7:53 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> The real issue is that exceptions thrown in static initialization cause unrecoverable problems with the classloader. The static block in Config would be changed into an init() method that gets called by the constructor. So the getInstance() method would fail until a connection could be established. See the link below for an example of the modified Config class.
> 
> https://github.com/PennState/directory-fortress-core-1/blob/feature/modifyBootstrapSingleton/src/main/java/org/apache/directory/fortress/core/util/Config.java#L75
> 
> Any class using Config would need modified to call getInstance first and not call it in a static context.

Sounds good.  

A few more questions:

1. How do you propose coding the connection retry logic?
2. The config object is used throughout the code, in static blocks, static variable initializations and such.
3. There is a complex interaction between the ApacheDsDataProvider and the Config objects.  The static block of the former is where the ldap connection pools are initialized.  That is kicked off by the latter.  Have you considered how to recode these classes?

Thanks

Shawn 

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
The real issue is that exceptions thrown in static initialization cause unrecoverable problems with the classloader. The static block in Config would be changed into an init() method that gets called by the constructor. So the getInstance() method would fail until a connection could be established. See the link below for an example of the modified Config class.

https://github.com/PennState/directory-fortress-core-1/blob/feature/modifyBootstrapSingleton/src/main/java/org/apache/directory/fortress/core/util/Config.java#L75

Any class using Config would need modified to call getInstance first and not call it in a static context.




----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, March 15, 2016 4:58:16 PM
Subject: Re: Static Config Initialization Problems

> On Mar 15, 2016, at 3:38 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> In this example, is your concern looking up the property? PropertiesConfiguration in Config is loaded only once, so I can't imagine that looking up a property is that inefficient.
> 
> If it is we could create a variable and a getter in Config, something like...
> 
> public String getAdminImplementation(){
>  if(adminClassName == null){
>    adminClassName = config.getProperty(GlobalIds.ADMIN_IMPLEMENTATION);
>  }
> 
>  return adminClassName;
> }
> 
> then in the factory
> 
> String adminClassName = Config.getInstance().getAdminImplementation();

I misunderstood your approach.  I thought you were saying each manager should gets its own instance of a Config object which would load all of the properties again and be heavy. 

Looking up a few properties is not a concern.  I am wondering how this will help the system recover.  If the objective is to (re)connect to an ldap server for example after bootstrap.

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 15, 2016, at 3:38 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> In this example, is your concern looking up the property? PropertiesConfiguration in Config is loaded only once, so I can't imagine that looking up a property is that inefficient.
> 
> If it is we could create a variable and a getter in Config, something like...
> 
> public String getAdminImplementation(){
>  if(adminClassName == null){
>    adminClassName = config.getProperty(GlobalIds.ADMIN_IMPLEMENTATION);
>  }
> 
>  return adminClassName;
> }
> 
> then in the factory
> 
> String adminClassName = Config.getInstance().getAdminImplementation();

I misunderstood your approach.  I thought you were saying each manager should gets its own instance of a Config object which would load all of the properties again and be heavy. 

Looking up a few properties is not a concern.  I am wondering how this will help the system recover.  If the objective is to (re)connect to an ldap server for example after bootstrap.



Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
In this example, is your concern looking up the property? PropertiesConfiguration in Config is loaded only once, so I can't imagine that looking up a property is that inefficient.

If it is we could create a variable and a getter in Config, something like...

public String getAdminImplementation(){
  if(adminClassName == null){
    adminClassName = config.getProperty(GlobalIds.ADMIN_IMPLEMENTATION);
  }
  
  return adminClassName;
}

then in the factory

String adminClassName = Config.getInstance().getAdminImplementation();



----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, March 15, 2016 12:24:51 PM
Subject: Re: Static Config Initialization Problems

Apologize for delay responding, am traveling atm….

> On Mar 12, 2016, at 7:41 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Yes, the actual problem is that fortress can't recover.
> 
> There are probably a number of ways to approach this... One simple way I was able to get working was to change singleton pattern in Config.java to
> 
>    private static Config INSTANCE = null;
> 
>    public static Config getInstance() {
>        if(INSTANCE == null) {
>        	LOG.info("Creating new instance");
>        	INSTANCE = new Config();
>        }
>        return INSTANCE;
>     }
> 
> then in one of the factories, not loading the class name statically
> 
>    public static AdminMgr createInstance(String contextId)
>        throws SecurityException
>    {
>    	String adminClassName = Config.getInstance().getProperty(GlobalIds.ADMIN_IMPLEMENTATION);

With this approach we’re going to be adding weight to the instantiation of the manger impl which up to now have been very light.  My concern are usage patters where these managers are created and discarded and don’t need to be held onto.

Shawn

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
Apologize for delay responding, am traveling atm….

> On Mar 12, 2016, at 7:41 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Yes, the actual problem is that fortress can't recover.
> 
> There are probably a number of ways to approach this... One simple way I was able to get working was to change singleton pattern in Config.java to
> 
>    private static Config INSTANCE = null;
> 
>    public static Config getInstance() {
>        if(INSTANCE == null) {
>        	LOG.info("Creating new instance");
>        	INSTANCE = new Config();
>        }
>        return INSTANCE;
>     }
> 
> then in one of the factories, not loading the class name statically
> 
>    public static AdminMgr createInstance(String contextId)
>        throws SecurityException
>    {
>    	String adminClassName = Config.getInstance().getProperty(GlobalIds.ADMIN_IMPLEMENTATION);

With this approach we’re going to be adding weight to the instantiation of the manger impl which up to now have been very light.  My concern are usage patters where these managers are created and discarded and don’t need to be held onto.

Shawn

Re: Static Config Initialization Problems

Posted by Chris Pike <cl...@psu.edu>.
Yes, the actual problem is that fortress can't recover.

There are probably a number of ways to approach this... One simple way I was able to get working was to change singleton pattern in Config.java to

    private static Config INSTANCE = null;
    
    public static Config getInstance() {
        if(INSTANCE == null) {
        	LOG.info("Creating new instance");
        	INSTANCE = new Config();
        }
        return INSTANCE;
     }

then in one of the factories, not loading the class name statically

    public static AdminMgr createInstance(String contextId)
        throws SecurityException
    {
    	String adminClassName = Config.getInstance().getProperty(GlobalIds.ADMIN_IMPLEMENTATION);
    	





----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Saturday, March 12, 2016 5:24:32 AM
Subject: Re: Static Config Initialization Problems

> On Mar 11, 2016, at 7:01 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> We need to handle the config initialization differently. See the static init block in this class...
> 
> https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/util/Config.java
> 
> If this fails when the application starts, it throws an exception and is never reinitialized. This results in all further requests throwing NoClassDefFoundError (http://stackoverflow.com/questions/6352215/java-why-java-lang-noclassdeffounderror-caused-by-static-field-initializ-failur)

Chris,

It’s good that your eyeballs are here.  Using a singleton to store config data is a caveat of this system. 

As such there’s been an open ticket wrt fortress config using a singleton for some time.  The thing is the core depends on that class being properly instantiated, and config data loaded - early.  Any changes here will reverberate across the system as a whole.

Having said that I don’t disagree that it needs changing, but also want to be careful.

In your comments above you point out the symptom of the problem, which is a misleading exception that occurs when something goes wrong during bootstrap which complicates problem resolution - if you’ve never seen it before.

The actual problem is that fortress can’t recover once something goes wrong during config initialization - right?

So with that problem description, and my concerns being duly noted, what are your recommendations, i.e. how would you load the config data, and when?

Here’s some background on how the config system works.  We’ll need to preserve this capability, i.e. loading data from multiple sources:
https://github.com/apache/directory-fortress-core/blob/master/README-CONFIG.md

Thanks,

Shawn

Re: Static Config Initialization Problems

Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 11, 2016, at 7:01 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> We need to handle the config initialization differently. See the static init block in this class...
> 
> https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/util/Config.java
> 
> If this fails when the application starts, it throws an exception and is never reinitialized. This results in all further requests throwing NoClassDefFoundError (http://stackoverflow.com/questions/6352215/java-why-java-lang-noclassdeffounderror-caused-by-static-field-initializ-failur)

Chris,

It’s good that your eyeballs are here.  Using a singleton to store config data is a caveat of this system. 

As such there’s been an open ticket wrt fortress config using a singleton for some time.  The thing is the core depends on that class being properly instantiated, and config data loaded - early.  Any changes here will reverberate across the system as a whole.

Having said that I don’t disagree that it needs changing, but also want to be careful.

In your comments above you point out the symptom of the problem, which is a misleading exception that occurs when something goes wrong during bootstrap which complicates problem resolution - if you’ve never seen it before.

The actual problem is that fortress can’t recover once something goes wrong during config initialization - right?

So with that problem description, and my concerns being duly noted, what are your recommendations, i.e. how would you load the config data, and when?

Here’s some background on how the config system works.  We’ll need to preserve this capability, i.e. loading data from multiple sources:
https://github.com/apache/directory-fortress-core/blob/master/README-CONFIG.md

Thanks,

Shawn