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

[Jacob Wisor]

W dniu 22.01.2014 01:30, Jacob Wisor pisze:
> On 01/20/2014 09:11 PM, Andrew Azores wrote:
>> On 01/20/2014 02:50 PM, Andrew Azores wrote:
>>> On 01/20/2014 07:27 AM, Jiri Vanek wrote:
>>>> On 01/17/2014 10:08 PM, Andrew Azores wrote:
>>>>> (snip)
>>>>
>>>> Ouch. See my reply to patch.
>>>
>>> Didn't see any mail, but we discussed on IRC. Here's a recap:
>>> - The reflection hack is horrible. It doesn't quite work, and it's super ugly.
>>> - The process not exiting is definitely a problem. We should not be playing
>>> around with reflection on the policytool like this.
>>> - Calling PolicyTool.main() directly breaks the automated build(?). We don't
>>> want to make it a build dep, and it's too minor to bother with a configure
>>> step/flag(?).
>>> - Solution: Runtime#exec() makes its return, with a fallback method of
>>> reflectively invoking PolicyTool.main() if for any reason the exec fails
>>> (non-POSIX system, executable not on PATH, etc)
>>>
>>> So here's a patch implementing this solution, with lots of extra added
>>> documentation.
>>>
>>
>> And this time with better respect for event dispatch thread, I hope. (thanks
>> omajid)
>
> +    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. There is
> certainly no problem when anonymous class's methods and their enclosing methods
> share local variables running on the same thread. This case has been defined.
> But, in the case of threads, I would be rather careful. Sorry, I do not have the
> time to look it up in the specs. :-/

Well, I did look it up because it was quite an intriguing question. 
Sub-paragraph 3 of Java Language Specification §17.4.1 says it:
"Local variables (§14.4), formal method parameters (§8.4.1), and exception 
handler parameters (§14.20) are never shared between threads and are unaffected 
by the memory model." ;-)

> To get thread-safe, you would have to pass launchCommand as a parameter to
> Runnable's constructor and store its reference in a member variable.
>
> Btw, the preferred way to construct Process objects since JDK 1.5 is to use
> ProcessBuilder. ProcessBuilder would also allow you to set policytool's current
> working directory to user.home, which is advisable.
> [...]

Jacob