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

[Jie Kang]

----- 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, let alone two different applets. If we sync on resources, closing the executor service of a specific ResourceTracker after completion should be fine.

> 
> The solution in this direction will be to really shutdown the service once
> all resources are downloaded (in case of javaws)
> And in case of plugin be prepared once another appelt si opened, to create
> new service. But here I'm not sure how to handle multiple applets in time.
> Maybe another map like in jnlpclasslaoder?  I would really like to avoid
> this.
> >
> >>
> >>>
> >>> How about using a ResourceTracker Factory to create ResourceTrackers and
> >>> keep a list of them there. Then you can call the shutdown on all the
> >>> ResourceTrackers when IT-W exits.
> >>
> >> Again - when/where is that point where to call this "shutdown" ? Such a
> >> point
> >> do not exists in
> >> plugin, and is achiavable by many paths from javaws....
> >
> > JNLPRuntime:
> >
> >          Runtime.getRuntime().addShutdownHook(new
> >          Thread("JNLPRuntimeShutdownHookThread") {
> >              public void run() {
> >                  markNetxStopped();
> >                  CacheUtil.cleanCache();
> >              }
> >          });
> >
> > Does this shutdown hook not work?
> 
> Right now not. It is never called because jvm never shutdown.
> 
> But thanx for bringing it up - I would like to remove
> CacheUtil.cleanCache();
> It have no sense.

Please look into this into more detail before making that decision.

At the moment, the ResourceTracker marks out-of-date resources for deletion when updating resources. It doesn't clean them as it expects this hook to do so. Removing this from the hook means that out of date resources will stay in cache till the user manually removes them. You will need to edit other code to make sure the useless resources are deleted.


Regards,

> >
> >>
> >> Also - the service is static, so no need to track it...
> >>
> >> J:(
> >>
> >>
> >>
> >
> >
> > Regards,
> >
> >
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern