[rfc][icedtea-web] policytool in itweb-settings

[Jacob Wisor]

Although this thread has gotten off topic, I will answer one last time about this.

On 01/24/2014 07:08 PM, Andrew Haley wrote:
>> +    private static void policyToolLaunchHelper(final JFrame frame, final String
>> filePath) {
>> +        final String[] launchCommand = new String[] { "policytool", "-file",
>> filePath };
>>
>> Ah, the caveats of anonymous classes and threads again: The launchCommand local
>> constant is not guaranteed to be available when Runnable.run() is scheduled to run.
>>
>> +        new Thread(new Runnable() {
>> +            @Override
>> +            public void run() {
>> +                try {
>> +                    final Process ptool =
>> Runtime.getRuntime().exec(launchCommand);
>>
>> The launchCommand local constant referenced here which lives on the
>> stack of the anonymous class's enclosing policyToolLaunchHelper()
>> method is not guaranteed to be valid or to exist after the thread
>> had been started. policyToolLaunchHelper() may terminate and thus
>> its local stack be cleaned up /before/ Runnable.run() is scheduled
>> to run, leaving no constant for reference. I am not entirely sure
>> about what the Java Programming Language Spec and/or JVM Spec define
>> in this case but I would bet that it has intentionally been left
>> undefined.
>
> It hasn't.
>
> launchCommand is a local variable in the context of
> policyToolLaunchHelper.  It is a final local, so it cannot be changed.

Just because something is final it does not mean the compiler is free to make it 
any class's field, especially when threads are involved.

> It points to an array of String.  This array is not constant.
>
> The array will continue to exist for as long as there is a reference
> to it from any thread.  The anonymous thread we just created, of
> course, holds a reference to the array; it does not, and it cannot,
> hold a reference into the stack frame of policyToolLaunchHelper.  That
> really would be a violation of the JLS and the VM specification.
>
> With regard to javac generating magic fields to capture variables in
> scope, it's time for you to get used to that.  Closures, in
> particular, do it all the time.

Nevertheless, I am admitted to calling this a violation of the Java Language 
Spec for as long as "automagically" adding fields to classes for the sake of 
patching things up in order to get them running has not been declared a viable 
strategy for implementors of compilers in that very same spec.

>> I don't know whether this is a hack, but a compiler that does this
>> kind of implicit code generation seems to violate the Java Language
>> Spec imho. Simply because the Java Language Spec does not state
>> whether accessing local constant variables from an inner class on a
>> different thread is a legal construct or not, or what the side
>> effects of it are. Do I get to access this synthetic field,
>> e.g. using a different language than Java or when using this inner
>> class as a library? IMHO this violates the memory model specified in
>> §17.4, especially §17.4.1 for shared variables. Or, the spec is
>> perhaps not clear enough about this and should be amended in
>> §4.12.4, §8.1.4, and/or §17.4.1?
>
> You're confusing local variables shared between threads, which is
> impossible, and the values to which they point being shared, which is
> not.

No, I am not. As you laid out yourself, this reference (not the object) has to 
be stored *somewhere*. policyToolLaunchHelper()'s thread stores it on its stack 
for the duration of policyToolLaunchHelper(). The compiler synthesizes a field 
into the inner class to store this reference for its thread. This is not visible 
from the source code. If you would ask almost anyone what the properties of 
localCommand in the inner class are, he/she would answer that it is a shared 
local variable, not the inner class's field. It has not been stipulated by the 
Java Language Spec that accessing local constant variables of enclosing classes 
from inner classes makes them fields of that particular inner class. Unless you 
show me otherwise, I beg to differ. ;-)

Jacob