You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2007/04/23 17:31:37 UTC

DO NOT REPLY [Bug 42198] New: - Insufficient synchronization for CometEvent.close

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198

           Summary: Insufficient synchronization for CometEvent.close
           Product: Tomcat 6
           Version: 6.0.11
          Platform: PC
        OS/Version: Windows XP
            Status: NEW
          Severity: critical
          Priority: P2
         Component: Catalina
        AssignedTo: tomcat-dev@jakarta.apache.org
        ReportedBy: matthias.reich@siemens.com


I am currently porting our eventing framework for web based clients to Tomcat 6,
as we would like to make use of the new CometProcessor interface.

I detected that the concurrent access of event processor thread and response
provider thread seems to be unsufficiently synchronized.

I built a small test webapp which tries to simulate the communication behaviour
of our framework and did some tests.
(see attachment comettest.war)
You can easily use the test webapp by simply deploying it to the webapps folder
of Tomcat,
then start one more more browsers (IE and FireFox shown a little different
behaviour),
and request the index page of the webapp.

I did my tests first with tomcat 6.0.10,
then with http://svn.apache.org/repos/asf/tomcat/tc6.0.x/tags/TOMCAT_6_0_11,
revision 530531, and finally with the trunk, revision 531159.
For the APR connector I used tcnative-1.dll in version 1.1.8.

Without enhanced synchronization, the XMLHttpRequests are often hanging after
reaching readyState 3 (often with Firefox, sometimes with IE).
Furthermore, CometEventImpl.close sometimes fails, and with the NIO connector I
saw e.g. this Exception:

java.lang.NullPointerException
	at
org.apache.coyote.http11.InternalNioOutputBuffer.writeToSocket(InternalNioOutputBuffer.java:436)
	at
org.apache.coyote.http11.InternalNioOutputBuffer.flushBuffer(InternalNioOutputBuffer.java:761)
	at
org.apache.coyote.http11.InternalNioOutputBuffer.endRequest(InternalNioOutputBuffer.java:398)
	at org.apache.coyote.http11.Http11NioProcessor.action(Http11NioProcessor.java:1087)
	at org.apache.coyote.Response.action(Response.java:183)
	at org.apache.coyote.Response.finish(Response.java:305)
	at org.apache.catalina.connector.OutputBuffer.close(OutputBuffer.java:276)
	at org.apache.catalina.connector.Response.finishResponse(Response.java:486)
	at org.apache.catalina.connector.CometEventImpl.close(CometEventImpl.java:85)
	at comettest.CometServlet.closeEvent(CometServlet.java:331)
	at comettest.CometServlet.access$2(CometServlet.java:317)
	at comettest.CometServlet$EventProvider.sendResponse(CometServlet.java:146)
	at comettest.CometServlet$EventProvider.run(CometServlet.java:95)
	at java.lang.Thread.run(Thread.java:595)


With the APR connector sometimes even the VM crashed. (see attached dump)


This is probably related to my observation that, if the concurrent close is
executed immediately after the CoyoteAdapter.service call, there is not END
event signalled.
Also with the NIO connector I saw that I did not get END events for all BEGIN
events.

Thus, my guess was that response objects might have been recycled too early, and
I modified the classes org.apache.catalina.connector.CometEventImpl,
org.apache.catalina.connector.CoyoteAdapter, and
org.apache.catalina.connector.Request to add a synchronization between event
processor thread and response provider thread via the CometEventImpl object.
(see attachment patches-reich.jar)

This synchronization prevents a recycling of request and response before the
close operation is completely finished and ensures that the close operation is
executed at most once.
- If one Thread enters the close method, the state of the object changes from
OPEN to CLOSING. When the close has finished, state goes to CLOSED.
- One sync point is in the CoyoteAdapter.service method when decides whether the
request shall be closed or put into the CometPoller.
  If the CometEventImpl.close method has not been called until then (state is
still OPEN), any later invocation of close is allowed to perform recycling of
request and response object by its own, until a new event is dispatched for the
request.
- The second sync point is at event dispatching to the CoyoteAdapter.event method.
  If the CometEventImpl state is still OPEN, the CoyoteAdapter takes over
responsibilty for recycling again, until the end of event dispatching.
  If CometEventImpl state is CLOSED, it does not make sense to send an END event
into the valve, because CometEventImpl.close has already recycled request and
response object.

I found out that the asynchronous response providers must not call the close
method of the OutputStream directly.
They must only use the CometEvent.close method, as otherwise there are still
problems due to unsynchronized access (I did not retest this with the trunk yet).


An alternative solution would be to leave the job of recycling completely to
CoyoteAdapter.event, but then we must be 100% sure that we get an appropriate
event from the underlying connector.

In my mind, the far better alternative would be to leave all recycling
completely to the garbage collector, i.e. do not reuse any objects at all, to
avoid the problems that arise automatically when we leave the world of
synchronous request processing, but surely this cannot be done in a day.

With my adaptation, my test application runs without problems for quite a while
with both connectors if there are only short resonse times for the poll requests.
I still sometimes (but not that often) got VM crashes with the APR connector at
the same point of execution of my response provider thread.
I am not sure if my synchronization is still not good enough, or if these
crashes happened when I pressed the reload button to reload the test page into
the browser.
(I saw in the repository created by 'ant download' that you are already working
on tcnative 1.1.10, so that problem might already be solved.)

As I ran my tests (server and browsers) on a WinXP/SP2 machine with a single
processor, I cannot tell whether my adaptation also works well on other
operating systems or on multi processor machines. Furthermore, I don't know if
it really fits with your concepts of the CometEvent lifecycle model. Therefore,
I cannot claim that my adaptations are a patch.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From matthias.reich@siemens.com  2007-04-23 08:33 -------
Created an attachment (id=20017)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=20017&action=view)
Test Web application to reproduce bug


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198


matthias.reich@siemens.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20018|0                           |1
        is obsolete|                            |




-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From remm@apache.org  2007-04-23 08:51 -------
I think you should first be testing with the current svn code. To make things
short, I am not going to agree to some sync (it is up to you to sync most write
operations), and I don't know yet if your usage is legitimate or not.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From matthias.reich@siemens.com  2007-04-23 08:36 -------
Created an attachment (id=20018)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=20018&action=view)
My modifications of Tomcat6

perhaps this will be considered to be a patch, but I am not sure ...

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From matthias.reich@siemens.com  2007-04-23 08:38 -------
Created an attachment (id=20019)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=20019&action=view)
VM dump after APR segmentation fault


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From remm@apache.org  2007-04-23 17:36 -------
I think there are a few sync strategies which can work.

- I think there is no reason to stop using a single event instance
- the ChatServlet only has one main thread writing (the others are there to add
stuff to do), so there is no unnecessary contention
- event.close indeed does a close, but it may not make a real difference
(because the stream close also ends the response)
- in svn head, you will get an END event in certain cases (like if the event or
stream is closed asynchronously), but it is not required to close the event
again since it's been done already; if the request is closed synchronously
during processing of the event method, for example when processing a timeout,
you will not get another END event; there's a valve which provides additional
END events (on session expiration, etc)

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198


matthias.reich@siemens.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20017|Test Web application to     |comettest.war
        description|reproduce bug               |




------- Additional Comments From matthias.reich@siemens.com  2007-04-23 12:36 -------
(From update of attachment 20017)
Test Web application to reproduce bug


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198


matthias.reich@siemens.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20017|0                           |1
        is obsolete|                            |




------- Additional Comments From matthias.reich@siemens.com  2007-04-23 14:52 -------
Created an attachment (id=20025)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=20025&action=view)
Corrected version of comettest.war

You are right - it is not a bug! I synchronized access to the output stream by
synchronizing on the event object, and the synchronization problem is solved -
Thank you for your quick comment.

Synchronization on the CometEvent object works fine with the current
implementation which reuses the same CometEventImpl instance throughout the
lifetime of a request, but is this part of the contract between container and
the Servlet?

I would not like the idea of synchronizing write operations on lots of
connections with the help of a single synchronizer object like the example
ChatServlet does. An alternative would be to synchronize on the
HttpServletResponse object. What do you recommend?

As I understood your comment, event.close and outputStream.close are not
intended to be coupled as close as they are in the current implementation.

Thus, would it be the recommended use of the Comet interface if the response
provider thread closes only the stream but not the event, so that the Servlet's
event method will be triggered with an END event, and the event method can
close the event?
Will a close of the event be required in case of an END event (e.g. to enable
recycling of request and response) or is it optional?


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198


matthias.reich@siemens.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID




-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From remm@apache.org  2007-04-23 12:26 -------
Ok, I have read (it was a bit hidden) that you have tested with the current svn
code. I had forgotten that close was immediately closing the response. As the
thing is designed, this is a bit wrong, I think, so I will make an update.

I don't understand how your problem can happen at this time (it would imply that
the internal objects have been recycled, however), since I don't see anything
occuring on your connections. Maybe you could use the system property
org.apache.catalina.connector.RECYCLE_FACADES set to true (you can put it in
catalina.properties). It protects from crashes from funky async accesses (like
Java2D, or here, bad Comet access).

You appear to be storing connections in a queue for some async processing. In
the design, it is up to you to synchronize accesses as needed (as with the rest
of the Servlet API, nothing is thread safe): in the example ChatServlet - not
functional, but is meant to be an IO test - all async writes are done inside a
sync on an object which is also used to manipulate the list of connections. This
way, you cannot have a close occurring at the same time as another write (or
close). The more I think about it, the less I think there's a bug to fix.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From remm@apache.org  2007-04-24 16:05 -------
This should be posted on the user list.

I doubt what you described corresponds to what actually happens (with the
exception of write being a noop after closing the stream - it is the same in a
regular servlet).

If you are playing with the Connection header, the client could assume things
and abruptly close the connection. You should not be playing with the connection
state (it's also true in a regular servlet), but it should not make any
difference as long as the client does not get confused.

After the END event, it is clearly mentioned that the request is done, and
objects will be recycled, so it's up to you to stop doing things with that
request (if you want to code more freely, you should use
org.apache.catalina.connector.RECYCLE_FACADES set to true, which will avoid all
invalid accesses).

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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


DO NOT REPLY [Bug 42198] - Insufficient synchronization for CometEvent.close

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=42198>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=42198





------- Additional Comments From daniel.doubleday@gmx.net  2007-04-24 14:04 -------
Hi, I know that this is no discussion forum but I wanted to comment on

(In reply to comment #8)
> - in svn head, you will get an END event in certain cases (like if the event or
> stream is closed asynchronously), but it is not required to close the event
> again since it's been done already; if the request is closed synchronously ...

If and when you get an END event depends very much on the connection type you
are using.

When you are using a Keep-Alive connection you dont get an END event when
calling event.close() synchronously. You will get it at some point later when
the browser decides to release the connection.

Otherwise (Connection: close) the connection will be released instantly and an
END event will be triggered. In that case the comet event object will become
kind of invalid, because if you call event.close() a NullPointerException will
by thrown. Unfortunately there is no way to query the event if it is already
closed. 

Personally I find that the event.close() method should never fail in that way
but rather return silently when being called on a closed event.

I wonder if there is a way to close the connection from the server side (other
than using Connection: close).

It's also a bit strange that you can keep on writing into a response output
stream of a closed event without getting an error when using Keep-Alive
connections. It seems to me that an exception should be thrown when trying to
write. 

Just my 5C


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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