[icedtea-web] not closeable javaws (and maybe more)
Jie Kang
jkang at redhat.com
Thu Feb 19 15:12:07 UTC 2015
----- 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?
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...
>
> > let alone two different applets. If we sync on resources, closing the
> > executor service of a specific ResourceTracker after completion should be
> > fine.
>
> Should be not - becasue of plugin and above. If you will came with working
> *and* well tested (on parallel and nearly parallel loading applets) patch,
> then we can continue in this direction.
>
> Currently I really see it like dangerous approach.
>
>
> J
>
>
> >
> >>
> >> 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.
> >
>
> Yup.You are done. Maybe method should be renamed? Maybe I got wrog impression
> because of not working current ctermination. Will look into this closer once
> the resourcesare resolved.
> >
> > Regards,
> >
> >>>
> >>>>
> >>>> Also - the service is static, so no need to track it...
> >>>>
> >>>> J:(
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>> Regards,
> >>>
> >>>
> >>
> >>
> >
>
>
--
Jie Kang
OpenJDK Team - Software Engineering Intern
More information about the distro-pkg-dev
mailing list