[icedtea-web] not closeable javaws (and maybe more)

[Jiri Vanek]

On 02/19/2015 04:12 PM, Jie Kang wrote:
>
>
> ----- Original Message -----
>> On 02/19/2015 03:14 PM, Jie Kang wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> On 02/18/2015 08:00 PM, Jie Kang wrote:
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>>> On 02/18/2015 03:23 PM, Jie Kang wrote:
>>>>>>>
>>>>>>>
>>>>>>> ----- Original Message -----
>>>>>>>> On 02/17/2015 11:57 AM, Jiri Vanek wrote:
>>>>>>>>> On 02/17/2015 11:00 AM, Jiri Vanek wrote:
>>>>>>>>>> Hi!
>>>>>>>>>>
>>>>>>>>>> Head, for already some time javaws apps needs to be closed by signal
>>>>>>>>>> kill.
>>>>>>>>>> For long time I thought this is soem accident or whatever, but it is
>>>>>>>>>> not.
>>>>>>>>>> Today I tracked it to guilty changeset of
>>>>>>>>>> http://icedtea.classpath.org/hg/icedtea-web/rev/6f4c1d501560
>>>>>>>>>>
>>>>>>>>>> Looking for the fix now, but in case I fail, this will spare some
>>>>>>>>>> time
>>>>>>>>>> to
>>>>>>>>>> the followr,
>>>>>>>>>>
>>>>>>>>>>       J.
>>>>>>>>>
>>>>>>>>> Ok, so the caus eis clear, ServiceExecutor do not run as daemon, and
>>>>>>>>> so
>>>>>>>>> is
>>>>>>>>> preventing jvm to stop.
>>>>>>>>>
>>>>>>>>> I'm wondering Why I dont see this issue from plugin side...
>>>>>>>>>
>>>>>>>>> Anyway - for javaws - both ok and failed run - where to place call to
>>>>>>>>>
>>>>>>>>> diff -r a8baec8d9d21
>>>>>>>>> netx/net/sourceforge/jnlp/cache/ResourceTracker.java
>>>>>>>>> --- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java    Fri Feb
>>>>>>>>> 13
>>>>>>>>> 12:48:24 2015 +0100
>>>>>>>>> +++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java    Tue Feb
>>>>>>>>> 17
>>>>>>>>> 11:57:01 2015 +0100
>>>>>>>>> @@ -34,6 +34,7 @@
>>>>>>>>>       import java.util.List;
>>>>>>>>>       import java.util.concurrent.ExecutorService;
>>>>>>>>>       import java.util.concurrent.Executors;
>>>>>>>>> +import java.util.concurrent.TimeUnit;
>>>>>>>>>
>>>>>>>>>       import net.sourceforge.jnlp.DownloadOptions;
>>>>>>>>>       import net.sourceforge.jnlp.Version;
>>>>>>>>> @@ -633,4 +634,15 @@
>>>>>>>>>           interface Filter<T> {
>>>>>>>>>               public boolean test(T t);
>>>>>>>>>           }
>>>>>>>>> +
>>>>>>>>> +    public static void shutdDownThreadPool() throws
>>>>>>>>> InterruptedException{
>>>>>>>>> +        threadPool.shutdown();
>>>>>>>>> +        if (!threadPool.awaitTermination(5, TimeUnit.SECONDS)) {
>>>>>>>>> +            OutputController.getLogger().log("Executor did not
>>>>>>>>> terminate
>>>>>>>>> in the specified time.");
>>>>>>>>> +            List<Runnable> droppedTasks = threadPool.shutdownNow();
>>>>>>>>> +            OutputController.getLogger().log("Executor was abruptly
>>>>>>>>> shut
>>>>>>>>> down. " + droppedTasks.size() + " tasks will not be executed.");
>>>>>>>>> +        }
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +
>>>>>>>>>       }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ?
>>>>>>>>
>>>>>>>> One more thought, attached patch fixed the issue for me:
>>>>>>>>
>>>>>>>> It does only thing - it changes threads used by ExecutorService to
>>>>>>>> daemons.
>>>>>>>>
>>>>>>>> Well, I dont know whot Iahve done - I dont know if ExecutorService is
>>>>>>>> still
>>>>>>>> keeping its purpose when its threads are daemons.
>>>>>>>>
>>>>>>>> On contrary, documentation is silent, and usage of custom
>>>>>>>> ThreadFactory
>>>>>>>> is
>>>>>>>> supported....
>>>>>>>> As negativism, original code (before patch 6f4c1d501560) was not using
>>>>>>>> daemons. But was not recycling threads...
>>>>>>>>
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> I strongly prefer a solution that uses threadPool.shutdown() since it
>>>>>>> is
>>>>>>> the proper way to stop the service and generally much more
>>>>>>> predictable/safer solution.
>>>>>>
>>>>>> I'm the opposite. I consider this as much safer ad more predictable
>>>>>> approach...
>>>>>
>>>>>
>>>>> By making them daemon threads they are discarded on exit. How is this
>>>>> EVER
>>>>> safe or predictable?
>>>>>
>>>>>
>>>>> [1] http://crunchify.com/what-is-daemon-thread-in-java-example-attached/
>>>>> [2]
>>>>> http://stackoverflow.com/questions/2213340/what-is-daemon-thread-in-java
>>>>>
>>>>> "stacks are not unwound - the JVM just exits", "finally blocks are not
>>>>> executed"
>>>>
>>>> Well - we are syncing on resources, and waiting until they are all done.
>>>> Once
>>>> they are done, jnlpapp/applet is  launched.
>>>>
>>>> So any time later, we can safely expect them done.
>>>>
>>>> *but* because of plugin, we can not terminate the service in this point.
>>>> The
>>>> service will be resurected once another applet is started.
>>>
>>> We don't reuse the same ResourceTracker object within the same applet,
>> ??? May you elaborate a bit? I have to miss something.
>>
>> The servicexecutor is static, and it is in shared (main) classlaoder. It is
>> reused.
>
> My mistake, I thought the executor was a object field.
>
>
>      private static final Object lock = new Object(); // used to lock static structures
>
>      private static final ExecutorService threadPool = Executors.newCachedThreadPool();
>
> So can't we make these not static?

It will fall into same probelms as I described .
>
>
> Anyways, if you'd like, please post the daemon-thread patch into [rfc] and we can review it from there. Maybe someone else will have thoughts...

Hmm. If you insists...
>
>
>
J.