[jira] Commented: (TAPESTRY-230) Ignored interruptedException prevents Janitorthread termination

["David Taylor (JIRA)" <ta...@jakarta.apache.org>]

     [ http://issues.apache.org/jira/browse/TAPESTRY-230?page=comments#action_60687 ]
     
David Taylor commented on TAPESTRY-230:
---------------------------------------

Here is my attempt at a fix based on the previous comments. This seems to fix the severe locking issue I was seeing when deploying to Tomcat 5.5.7 using MyEclipse. Independent verification would be appreciated since I have not done extensive testing.

A key observation not previously mentioned is that the thread interrupt() method has no effect when the thread is not sleeping. The use of a boolean flag solves this problem. The tricky part is that synchronization needs to be done properly to ensure the JMM forces memory cache flushing across the threads. Note the use of the volatile and synchronized keywords. The Java threading and memory management models have some very surprising wrinkles if you are used to threading on other platforms. IBM published a great article on this a few years back as I recall.

Anyway, if I haven't bored you to death, here are my simple code changes:


Changes to JanitorThread.java (v3.0.2)

Additions:

    /**
     *  Flag used to signal the thread to stop. Note: The volatile
     *  keyword may not be fully implemented on some JVMs
     */
    private volatile boolean terminated = false;

    /**
     * Signals the thread to terminate
     */

    public synchronized void terminate()
    {
         terminated = true;  // Always do this first!
         interrupt();
    }

Modifications:

    public void run()
    {
        while (true)
        {
            synchronized (this) // Must synchronize even when reading!
            {
                if (terminated)
                    return;
            }

            if (!waitForNextPass())
                return;

            sweep();
        }
    }


Changes to ApplicationServlet.java (v3.0.2)

Additions:


    /**
     *  Signals the shared janitor thread to terminate when the
     *  servlet is shutdown.
     */

    public void destroy() {
        try{
            // Close Tapestry JanitorThread
            JanitorThread janitorThread =           
                JanitorThread.getSharedJanitorThread();

            if (janitorThread != null)  // Just being a little paranoid
                janitorThread.terminate();
        } finally{
            super.destroy();
        }
   }


> Ignored interruptedException prevents Janitorthread termination
> ---------------------------------------------------------------
>
>          Key: TAPESTRY-230
>          URL: http://issues.apache.org/jira/browse/TAPESTRY-230
>      Project: Tapestry
>         Type: Bug
>   Components: Framework
>     Versions: 3.0, 3.0.1
>  Environment: Tomcat 4 & 5 (on Windows platform)
>     Reporter: Martijn Stellinga
>     Assignee: Howard M. Lewis Ship
>     Priority: Minor
>      Fix For: 3.0.2

>
> The org.apache.tapestry.util.JanitorThread uses a waitForNextPass() function that makes it sleep for 30 seconds and then do another pass.
> In this method an InterruptedException is caught and ignored. This prevents the thread from terminating normally when, for instance, a context is reloaded. See also http://java.sun.com/j2se/1.5.0/docs/guide/misc/threadPrimitiveDeprecation.html.
> I think it would be better if the while-loop in the run() method exited when the wait() in waitfornextpass() method was interrupted.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-dev-help@jakarta.apache.org