[RFC] netx: show security dialogs in a separate AppContext

[Omair Majid]

On 10/20/2010 02:45 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com>  [2010-10-20 14:09]:
>> Hi,
>>
>> On 10/19/2010 06:38 PM, Deepak Bhole wrote:
>>> * Omair Majid<omajid at redhat.com>   [2010-10-19 12:10]:
>>>> Hi,
>>>>
>>>> The attached patch makes security prompts appear in a separate
>>>> AppContext. This ensures that the applet's look and feel is not
>>>> automatically set to the system look and feel.
>>>>
>>>> The patch is a superset of http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2010-October/010500.html
>>>>
>>>> This patch also implements a large part of the code necessary to
>>>> allow applets to access their EventQueues. However, I still have to
>>>> figure out how to use
>>>> http://hg.openjdk.java.net/jdk7/awt/jdk/rev/8022709a306d to ensure
>>>> allowing access to the EventQueue is safe. So for now, that code is
>>>> disabled.
>>>>
>>>> The patch ensures that the JNLPRuntime is always initialized in the
>>>> main AppContext. It starts a thread in this AppContext to listen for
>>>> security dialog messages, and shows security dialogs when it
>>>> receives an appropriate security message. The security dialogs block
>>>> the client applet/application until the user responds.
>>>>
>>>> Any thoughts or comments?
>>>>
>>>
>>> Hi,
>>>
>>> Please see comments below.
>>>
>>
>> Thanks for the review and the comments. Updated patch attached.
>>
>>> Cheers,
>>> Deepak
>>>
>>>> Thanks,
>>>> Omair
>>>
> <snip>
>>>> +final class SecurityDialogMessage {
>>>> +
>>>> +    /*
>>>> +     * These fields contain information need to display the correct dialog type
>>>> +     */
>>>> +
>>>> +    public DialogType dialogType;
>>>> +    public AccessType accessType;
>>>> +    public JNLPFile file;
>>>> +    public CertVerifier certVerifier;
>>>> +    public X509Certificate certificate;
>>>> +    public Object[] extras;
>>>> +
>>>> +    public volatile Object userResponse;
>>>> +
>>>
>>> Why is userResponse volatile? Based on the patch, for a given instance,
>>> only one thread (either the even dispatcher when action is performed OR
>>> another caller thread when locking throws an interruptedexception)
>>> modifies the value... or am I missing something?
>>>
>>
>> I apologise in advance if my knowledge of concurrency is a little
>> off, but we need to ensure that setting the values on the message
>> object happen-before reading it. Yes, only one thread can change it,
>> but the change needs to propogate to the other thread. The lock
>> ensures that this happens in the non EventDispatchThread case. In
>> the case of the EventDispatchThread, one thread sets the value and
>> the other reads it. The volatile ensures that changes to userReponse
>> are visible to the client thread.
>>
>> Perhaps I am wrong; I can make it non-volatile if you want me to.
>>
>
> Ah I see. Yes, you are correct. Volatile in this case will ensure that
> userResponse is not locally cached, and therefore the EDT will always
> see the update value when returning from getUserResponse().
>

I added a comment to userReponse explaining this - it should make it 
more clear.

> Okay, everything else looks good to me. Okay for commit to head.
>

Thanks for the review! Committed as changeset eb998ed0ab1a.

Cheers,
Omair