[rfc] [icedtea-web] singletons logic, logs and test cleanup/fixes

[Andrew Azores]

On 12/17/2013 10:29 AM, Jiri Vanek wrote:
> hi!
>
> Motivation - after "console aware of plugin" patch, some tests started 
> *sometimes* die as "can not intialize x11" or "null" (not NPE, but 
> just "null" it was that test was null - so class was not even 
> statically instantiated as I found later).
>
> Imho the "console aware of plugin" was not the cause, but make this 
> error more visible. Unfortunately i have not found the exact cause, 
> but made few changes which make logic more direct and from those 
> (five) I think two were the cause of "null" and "x11" errors.
>
> The rewritten console pathc on review have to be adapted to this.
>
> fix 1: initialisation of console gui done in time when gui is shown, 
> not when console is first time "used" - this is crucial for "x11" error.
>
> fix2: the NoStdOutErrTest.java methods do not throw any Exception now. 
> Instead of this they log it and pretend to be ok. This was crucial for 
> "null" error. Sometimes race condition or "x11" error (or null was 
> causing x111 ???) throw exception here, which confused junit bloody 
> much. a) it was consumed b)test was not working

Good!

>
>
> fix3: removed internal MessageWithLevel class. It lost it sense with 
> addition of headers.

Nice :)

>
> fix4: debug messages from plugin are not going directly to console 
> output, but are buffered in already exisiting messageQue. I'm not sure 
> how good idea it is. It can lower the synchronisation issues.

Hmm... I think this is okay. It makes sense to use the existing queue.

>
> fix5: all static factory methods made synchronised. Imho it is the 
> only correct, but.... :) Also I made the synchronisation via 
> "synchronised" but I'm not sure how correct it is... Maybe via static 
> final Object lock = new Object(); is better?
>

I think using the synchronized keyword is good enough here. If somehow 
we find later on that it's causing problems then it can be fixed, but I 
don't think it should be an issue.

Ok for head IMO.


Thanks,

-- 
Andrew A