[rfc][icedtea-web] tuning of properties loading

[Jiri Vanek]

On 01/22/2014 11:05 PM, Andrew Azores wrote:
> On 01/22/2014 01:37 PM, Jiri Vanek wrote:
>> By hitting https://bugzilla.redhat.com/show_bug.cgi?id i realized, that we made an serious error
>> by not dying on ConfigureException.
>> This patch is fixing it. It also fix an bug, when deployment.system.config.mandatory is true, and
>> invlaid url is proposed.
>> It also add few cosmetic changes
>>  - few docs
>>  - better error reporting from loading the global config
>>  - better handling of non fatal (==non ConfigureException) ) exception
>>    - logged
>>    - defautls are used
>>    - if it is not processed, then instantiation error is thrown instead, which is not much helpful...
>>
>> The reproducers for this are hardly to be possible, as both global files needs root permissions.
>> However some unitsts will be hopefully possible.
>>
>> Thanx,
>>   J.
>
> +            if (systemPropertiesMandatory == true){
> +                throw new ConfigurationException("Invalid url to system properties, which are
> mandatory");
>
>
> " == true" ? ;)

  fixed.

>
>           } catch (IOException e) {
> +            if (mandatory){
> +                throw new ConfigurationException(e.getMessage());//unluckily
> ConfigurationException can not have cause
> +            }
>
>
> ConfigurationException does have initCause(Throwable), does this not match what you need? It gets
> this from its superclass NamingException which implements Throwable.

Ah I overlooked!. Tahnx. Fixed.
>
> +                //to be sure - we MUST die
> -http://docs.oracle.com/javase/6/docs/technotes/guides/deployment/deployment-guide/properties.html
> +            }catch(Throwable t){
> +                //all exceptions are causing InstantiatizationError so this do it much more readble
> +                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, t);
> +                OutputController.getLogger().log(OutputController.Level.WARNING_ALL,
> getMessage("RFailingToDefault"));
> +                //try to survive this unlikely exception
> +                config.resetToDefaults();
>
>
> Hmm, what's the motivation behind adding this? What is there that's causing InstantiationErrors [1],
> and if these occur, should it be trying to survive? Usually you don't want to be catching Errors
> [2], so catch-all Throwable is even worse than catch-all Exception.

a) the NoClassDefFoundError error is fatal and non recoverable
b) no solid info *why* it happened  appeared (consumed any NoClassDefFoundError?)
c) we can run pretty well with defaults (and msut warn user - I'm thinking also about poping up an 
dialogue in non headless mode, but I'm afraid that it cnabe to early to (maybe another chnageset)

I do not insist on throwable, If you wish I can lower to general exception or whatever you wont. But 
defintley do not wont this static block to throw something

>
> [1] http://docs.oracle.com/javase/7/docs/api/java/lang/InstantiationError.html

Ugh sorry - it is java.lang.NoClassDefFoundError: Could not initialize class 
net.sourceforge.jnlp.runtime.JNLPRuntime$DeploymentConfigurationHolder what is thrown when static 
initializer throw some excption out.

> [2] http://docs.oracle.com/javase/7/docs/api/java/lang/Error.html
>

  Thanks,
  J

-------------- next part --------------
A non-text attachment was scrubbed...
Name: tuningOfPropertiesLoading2.patch
Type: text/x-patch
Size: 7720 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140123/dc9917e3/tuningOfPropertiesLoading2.patch