You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@xalan.apache.org by "Dave Brosius (JIRA)" <xa...@xml.apache.org> on 2005/12/04 17:37:29 UTC

[jira] Created: (XALANJ-2242) [PATCH] wait/notifying on a Thread object interferes with Thread behaviour

[PATCH] wait/notifying on a Thread object interferes with Thread behaviour
--------------------------------------------------------------------------

         Key: XALANJ-2242
         URL: http://issues.apache.org/jira/browse/XALANJ-2242
     Project: XalanJ2
        Type: Bug
  Components: Other  
    Versions: Latest Development Code    
    Reporter: Dave Brosius
    Priority: Minor


Calling wait or notify/notifyAll on the thread object itself, interferes with the basic Thread management built into the jvm, causing spurious thread states, including threads waking up when not expected, etc. 

This patch fixes this

-- 
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
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


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


[jira] Commented: (XALANJ-2242) [PATCH] wait/notifying on a Thread object interferes with Thread behaviour

Posted by "Kevin Cormier (JIRA)" <xa...@xml.apache.org>.
    [ http://issues.apache.org/jira/browse/XALANJ-2242?page=comments#action_12448260 ] 
            
Kevin Cormier commented on XALANJ-2242:
---------------------------------------

As far as I know, calling wait or notify/notifyAll on a thread object does not interfere with the JVM's thread management.  However, the synchronization in this applet is clearly incorrect.  That said, it does work for the most common case.

Unfortunately, the supplied patch does not fix the problems.  Since this is a minor issue, I don't think it should be fixed before the 2.7.1 release.  The following is additional information on the synchronization problems and how we might address them in the future.

The applet has no GUI, but can be used with Javascript (via LiveConnect) to provide XSLT services to a web page.  (There is an example of this in samples/AppletXMLtoHTML.)  To overcome security restrictions that apply to LiveConnect threads, the applet uses it's own thread (TrustedAgent) that polls a boolean instance variable which is used to indicate that a request has been made.  TrustedAgent handles two types of requests - getting the contents of the input documents (XML and XSL), and performing a transformation.

There are at least 3 synchronization problems:

1. The thread requesting data sets the instance variable m_callThread to point to itself, and then calls wait() on itself.  Any other thread can change the value of m_callerThread, and the TrustedAgent would never call notify() on the first thread.

The patch attempts to fix this by introducing an Object on which to synchronize, but it does not address the remaining issues.

2.  wait() is not called in a loop.  wait() must always be called in a loop that checks whether a certain condition is met.  In this case, we need to check if the TrustedAgent is finished doing it's work.  If the thread requesting data is interrupted before arriving at the wait() call and the TrustedAgent completes its work and calls notify(), then the thread willwait forever.

3.  Access to the instance variables controlling the type of request is not synchronized.  If a transformation is requested, and then another thread requests the XSL document before the TrustedAgent begins the transformation, then that first request is ignored.  You can see this behaviour using the demo web page and applet.  Click on the button repeatedly, and occasionally you will see "null" in the result frame.  This is because when one of the input documents is fetched, it is stored in a different variable from the one used for the output document.  (Yes, each button click seems to get its own thread - there is no waiting for the handler function to complete.) 

The supplied patch does not fix this behaviour.  (Actually, the patch as-is is broken - you need to change all of the synchronization to use m_sync rather than m_callThread.)


It would be nice to clean up in the future though.  I prototyped a solution that addresses these problems though.  This one way to do it:

1.  Introduce a new class, say WorkItem, that has fields for describing the type of data request, indicating if the request has been processed, and storing the resulting String

2.  Introduce an instance variable that holds a thread-safe LinkedList to act as a queue - Collections.synchronizedList(new LinkedList()).  Threads requesting data add WorkItems to the queue, and the TrustedAgent removes and processes them.  Data-requesting threads wait on the request itself.  For example:

synchronized (request)
{
  while (!request.isComplete())
    request.wait();
}

And the TrustedAgent does this after it completes the request:

synchronized (request)
{
  request.setCompleted(true);
  request.notify();
}

3.  Add synchronization for other instance variables that are touched by threads (eg. m_documentURL & m_styleURL), or perhaps mark them as volatile and copy them into WorkItems so that the value used for a particular request is the value at the time the request was made.




> [PATCH] wait/notifying on a Thread object interferes with Thread behaviour
> --------------------------------------------------------------------------
>
>                 Key: XALANJ-2242
>                 URL: http://issues.apache.org/jira/browse/XALANJ-2242
>             Project: XalanJ2
>          Issue Type: Bug
>          Components: Other
>    Affects Versions: Latest Development Code
>            Reporter: Dave Brosius
>            Priority: Minor
>         Attachments: spurious_thread_states.diff
>
>
> Calling wait or notify/notifyAll on the thread object itself, interferes with the basic Thread management built into the jvm, causing spurious thread states, including threads waking up when not expected, etc. 
> This patch fixes this

-- 
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
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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


[jira] Commented: (XALANJ-2242) [PATCH] wait/notifying on a Thread object interferes with Thread behaviour

Posted by "Kevin Cormier (JIRA)" <xa...@xml.apache.org>.
    [ http://issues.apache.org/jira/browse/XALANJ-2242?page=comments#action_12448480 ] 
            
Kevin Cormier commented on XALANJ-2242:
---------------------------------------

java.lang.Thread calls wait() on this in the join() method.  It calls it in a loop, using isAlive() as the condition.  If we signal the thread, it wakes up, checks that isAlive() still returns true, and waits again.

On the other hand, since any one can join the thread, threads call notifyAll() when they exit (see ThreadGroup.remove()). So if we wait on the thread in our application, we will be signaled, but so will all of the other threads.

So we don't have any ill effects on the thread system by calling wait() or notify() on the thread itself.  If you can provide evidence to the contrary or point me to some documentation that says otherwise, I'd be interested to see it.

> [PATCH] wait/notifying on a Thread object interferes with Thread behaviour
> --------------------------------------------------------------------------
>
>                 Key: XALANJ-2242
>                 URL: http://issues.apache.org/jira/browse/XALANJ-2242
>             Project: XalanJ2
>          Issue Type: Bug
>          Components: Other
>    Affects Versions: Latest Development Code
>            Reporter: Dave Brosius
>            Priority: Minor
>         Attachments: spurious_thread_states.diff
>
>
> Calling wait or notify/notifyAll on the thread object itself, interferes with the basic Thread management built into the jvm, causing spurious thread states, including threads waking up when not expected, etc. 
> This patch fixes this

-- 
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
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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


[jira] Commented: (XALANJ-2242) [PATCH] wait/notifying on a Thread object interferes with Thread behaviour

Posted by "Dave Brosius (JIRA)" <xa...@xml.apache.org>.
    [ http://issues.apache.org/jira/browse/XALANJ-2242?page=comments#action_12448373 ] 
            
Dave Brosius commented on XALANJ-2242:
--------------------------------------

Sun's implemenation of java.lang.Thread calls wait and notify on 'this' for it's own purposes. (Which obviously is a bad design). If you call notify or wait on the thread as well, you will interfere with how the class works.



> [PATCH] wait/notifying on a Thread object interferes with Thread behaviour
> --------------------------------------------------------------------------
>
>                 Key: XALANJ-2242
>                 URL: http://issues.apache.org/jira/browse/XALANJ-2242
>             Project: XalanJ2
>          Issue Type: Bug
>          Components: Other
>    Affects Versions: Latest Development Code
>            Reporter: Dave Brosius
>            Priority: Minor
>         Attachments: spurious_thread_states.diff
>
>
> Calling wait or notify/notifyAll on the thread object itself, interferes with the basic Thread management built into the jvm, causing spurious thread states, including threads waking up when not expected, etc. 
> This patch fixes this

-- 
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
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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