You are viewing a plain text version of this content. The canonical link for it is here.
Posted to xmlrpc-auto@ws.apache.org by "Alan Burlison (JIRA)" <xm...@ws.apache.org> on 2009/04/28 11:47:30 UTC

[jira] Created: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

XmlRpcClient is supposed to be thread-safe but it isn't
-------------------------------------------------------

                 Key: XMLRPC-167
                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
             Project: XML-RPC
          Issue Type: Bug
          Components: Source
    Affects Versions: 3.1.2
         Environment: Solaris
            Reporter: Alan Burlison


http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:

----------
A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
----------

I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:

----------
Fatal Error] :1:1: Content is not allowed in prolog.
operation failed, Failed to parse server's response: Content is not allowed in prolog.
[Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
----------

----------
[Fatal Error] :1:1: Content is not allowed in prolog.
[Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Content is not allowed in prolog.
----------

If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.

It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707176#action_12707176 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

I'll check out the 1.4/1.5 issue, I thought I was seeing my code go through the 1.5 variant when I was stepping through it in the debugger, but I'll double check.

The issue is that the object returned by getTransport() isn't thread-safe, so if you share that returned object between two threads they can both end up using the same underlying URLconnection at the same time, and that's what causes the problem.  You can get into that situation in turn if you share the XmlRpcClient object that doles out the transports between two threads.

It may be the case that I'm setting up the XML-RPC 'stack' incorrectly in the first place, but the docs do say that XmlRpcClient is thread-safe.  The code that implements my client can be found at http://src.opensolaris.org/source/xref/website/auth/AuthClient/src/org/opensolaris/auth/client/AuthClient.java, it may be worth you taking a look at that to make sure I'm not violating some assumption that the XML-RPC library makes about how things are set up - it's entirely possible I am :-)

As for my 'real' app, it is doing more realistic processing in both the client & the server (crypto stuff), so the window for the race condition to occur is wider and therefore easier to hit. 

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703560#action_12703560 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Alan, this isn't proof that the client has a problem: For exactly the same reason, the server might break.

In other words, you should repeat your example, tracing the servers output.

Apart from that, the important information is missing, what transport factory you are using: Obviously thread safety is actually a problem of the transport factory.


> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707327#action_12707327 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

Well it could be made thread safe very easily, at least in the case of the Sun transport.  And if not, at very least the documentation needs changing, as saying that XmlRpcClient is thread-safe rather misleading, as it depends on the transport factory and transport it is configured to use.

In my case I assume the correct thing to do is to modify my transport factory so that it returns a new instance of the transport on each call to getTransport, rather than using a singleton as it does at present, is that correct?

With the exception of the addition of 'synchronized' to XmlRpcSunHttpTransport.sendRequest(), I think the rest of the patch can stand as it cleans up a few other issues, what do you think?

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12704307#action_12704307 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Alan, using the Client and Server from XMLRPC-168 (note: the updated version of the Client with 10 concurrent threads in parallel) I wasn't able to reproduce your problem. Consequently I doubt that the problem is in the XmlRpcClient or any related class.


> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707092#action_12707092 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Alan, your changes would make perfect sense to me, if TransportFactory.getTransport() would return a singleton object, or in similar circumstances. (I notice, you are using the XmlRpcSunHttpTransportFactory, as opposed to XmlRpcSun14 or XmlRpcSun15. Is that intentional, btw?)

However, that's not the case: Any call to getTransport() returns a new object, with a new URLconnection. So, what's the point with synchronizing sendRequest(), as the synchronized object is per-thread? And, in particular, the URLConnection is not "shared", as you state above?

I'd also like to understand what's the difference between my scenario from XMLRPC-168 and what you are doing to reproduce the problem and your "real app"?


> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706017#action_12706017 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

I've wrapped the InputStream read by XmpRpcStreamTransport with a simple class that just prints out the thread ID and the method to a passed output stream before passing the calls through to the wrapped class:

    InputSource isource = new InputSource(pStream);

becomes:

    InputSource isource = new InputSource(new InputStreamTracer(pStream, System.out, Thread.currentThread().getName()));
 
When I run my test client with two threads I get the following trace, which clearly shows that two threads are both trying to read the same reply from the server at the same time, hence the breakage.

Thread-0: read() = "<"
Thread-1: read() = "?"
Thread-0: read() = "x"
Thread-1: read() = "m"
Thread-0: read() = "l"
Thread-1: read() = " "
Thread-1: read() = "e"
Thread-0: read() = "v"
Thread-0: read(buff, 0, 28) = 28 "?><methodResponse><fault><va"
Thread-1: read(buff, 0, 28) = 28 "rsion="1.0" encoding="UTF-8""
[Fatal Error] :1:1: Content is not allowed in prolog.
[Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Content is not allowed in prolog.

There's clearly a race condition here, and clearly a missing lock around the input stream.

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707803#action_12707803 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Alan, give yourself the answer:

Suggest for a moment that we had a transport shared between threads and sendRequest were thread safe. Furthermore, suggest that we had a request running for 10 seconds.

As a consequence, no other request could run within these 10 seconds, at least not with this very transport. Do you think that's desirable?


> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707795#action_12707795 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

I *knew* I'd written my transport factor to return a singleton transport for a reason, this is from XmlRpcTransportFactory:

----------
    /** Returns an instance of {@link XmlRpcTransport}. This may
	 * be a singleton, but the caller should not depend on that:
	 * A new instance may as well be created for any request.
	 * @return The configured transport.
	 */
	public XmlRpcTransport getTransport();
----------

Jochen. that contradicts your statement that "the transport was never meant to be thread safe and will never be thread safe".  There is no way a transport factory can return an unsynchronized singleton and still be thread-safe.

I can either add the synchronization or fix the comment, but one or the other needs to be done.  Which would you prefer?

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707807#action_12707807 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

No I agree - just checking ;-)  So a patch to the docs is what you'd like, correct?

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707360#action_12707360 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Oh, and I forgot: In particular, I'd refuse to synchronize sendRequest. 

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707795#action_12707795 ] 

Alan Burlison edited comment on XMLRPC-167 at 5/10/09 7:57 AM:
---------------------------------------------------------------

I *knew* I'd written my transport factory to return a singleton transport for a reason, this is from XmlRpcTransportFactory:

----------
    /** Returns an instance of {@link XmlRpcTransport}. This may
	 * be a singleton, but the caller should not depend on that:
	 * A new instance may as well be created for any request.
	 * @return The configured transport.
	 */
	public XmlRpcTransport getTransport();
----------

Jochen. that contradicts your statement that "the transport was never meant to be thread safe and will never be thread safe".  There is no way a transport factory can return an unsynchronized singleton and still be thread-safe.

I can either add the synchronization or fix the comment, but one or the other needs to be done.  Which would you prefer?

      was (Author: alanbur):
    I *knew* I'd written my transport factor to return a singleton transport for a reason, this is from XmlRpcTransportFactory:

----------
    /** Returns an instance of {@link XmlRpcTransport}. This may
	 * be a singleton, but the caller should not depend on that:
	 * A new instance may as well be created for any request.
	 * @return The configured transport.
	 */
	public XmlRpcTransport getTransport();
----------

Jochen. that contradicts your statement that "the transport was never meant to be thread safe and will never be thread safe".  There is no way a transport factory can return an unsynchronized singleton and still be thread-safe.

I can either add the synchronization or fix the comment, but one or the other needs to be done.  Which would you prefer?
  
> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
     [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alan Burlison updated XMLRPC-167:
---------------------------------

    Attachment: XMLRPC-167.patch

Fixes the bug, also cleans up a few other minor issues I found along the way.  Making XmlRpcSunHttpTransport.sendRequest synchronized is the lowest point in the call stack that can be used to make the creation of the HttpURLConnection and its subsequent write & read operations atomic.

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707323#action_12707323 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Alan, the transport was never meant to be thread safe and will never be thread safe.


> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703595#action_12703595 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

I've looked at the server and it is seeing valid requests, and sending out responses.  I've modified the client so that it just creates a single thread and then run two instances against the server, and that works fine - well, up until the server deadlocks inside  org.apache.xmlrpc.util.ThreadPool, but that's the subject of a different bug...

The transport factor is a subclass of XmlRpcSunHttpTransport that overrides newURLConnection so that it uses a custom socket factory that creates SSL sockets, other than that it is completely standard.

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706549#action_12706549 ] 

Alan Burlison edited comment on XMLRPC-167 at 5/6/09 11:30 AM:
---------------------------------------------------------------

The attached patch (XMLRPC-167.patch) fixes the bug and also cleans up a few other minor issues I found along the way.  Making XmlRpcSunHttpTransport.sendRequest synchronized is the lowest point in the call stack that can be used to make the creation of the HttpURLConnection and its subsequent write & read operations atomic.

      was (Author: alanbur):
    Fixes the bug, also cleans up a few other minor issues I found along the way.  Making XmlRpcSunHttpTransport.sendRequest synchronized is the lowest point in the call stack that can be used to make the creation of the HttpURLConnection and its subsequent write & read operations atomic.
  
> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706017#action_12706017 ] 

Alan Burlison edited comment on XMLRPC-167 at 5/5/09 5:52 AM:
--------------------------------------------------------------

I've wrapped the InputStream read by XmpRpcStreamTransport with a simple class that for each method call prints out the thread ID, the method name, arguments and return value to a specified output stream:

    InputSource isource = new InputSource(pStream);

becomes:

    InputSource isource = new InputSource(new InputStreamTracer(pStream, System.out,
     Thread.currentThread().getName()));
 
When I run my test client with two threads I get the following trace, which clearly shows that two threads are both trying to read the same reply from the server at the same time, hence the breakage.

Thread-0: read() = "<"
Thread-1: read() = "?"
Thread-0: read() = "x"
Thread-1: read() = "m"
Thread-0: read() = "l"
Thread-1: read() = " "
Thread-1: read() = "e"
Thread-0: read() = "v"
Thread-0: read(buff, 0, 28) = 28 "?><methodResponse><fault><va"
Thread-1: read(buff, 0, 28) = 28 "rsion="1.0" encoding="UTF-8""
[Fatal Error] :1:1: Content is not allowed in prolog.
[Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Content is not allowed in prolog.

There's a race condition here, and what looks like a missing lock around the input stream.

      was (Author: alanbur):
    I've wrapped the InputStream read by XmpRpcStreamTransport with a simple class that just prints out the thread ID and the method to a passed output stream before passing the calls through to the wrapped class:

    InputSource isource = new InputSource(pStream);

becomes:

    InputSource isource = new InputSource(new InputStreamTracer(pStream, System.out, Thread.currentThread().getName()));
 
When I run my test client with two threads I get the following trace, which clearly shows that two threads are both trying to read the same reply from the server at the same time, hence the breakage.

Thread-0: read() = "<"
Thread-1: read() = "?"
Thread-0: read() = "x"
Thread-1: read() = "m"
Thread-0: read() = "l"
Thread-1: read() = " "
Thread-1: read() = "e"
Thread-0: read() = "v"
Thread-0: read(buff, 0, 28) = 28 "?><methodResponse><fault><va"
Thread-1: read(buff, 0, 28) = 28 "rsion="1.0" encoding="UTF-8""
[Fatal Error] :1:1: Content is not allowed in prolog.
[Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
operation failed, Failed to parse server's response: Content is not allowed in prolog.

There's clearly a race condition here, and clearly a missing lock around the input stream.
  
> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
     [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alan Burlison updated XMLRPC-167:
---------------------------------

    Attachment:     (was: XMLRPC-167.patch)

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706364#action_12706364 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

I've made XmlRpcStreamTransport.sendRequest synchronized as it is responsible for sending the request to the server and receiving the response.  That fixes the concurrent read issue, but reveals another problem:

----------
Thread-0: InputStreamTracer(sun.net.www.protocol.http.HttpURLConnection$HttpInputStream@eac5a)
Thread-0: read() = "<"
Thread-0: read() = "?"
Thread-0: read() = "x"
Thread-0: read() = "m"
Thread-0: read(buff, 0, 28) = 28 "l version="1.0" encoding="UT"
Thread-0: read() = "F"
Thread-0: read() = "-"
Thread-0: read() = "8"
Thread-0: read() = """
Thread-0: read() = "?"
Thread-0: read() = ">"
Thread-0: read(buff, 0, 8192) = 754 "<methodResponse><params><param><value><struct><member><name>name</name><value>Thread-0</value></member> ..."
Thread-0: read(buff, 0, 8192) = -1
Thread-0: close()
Thread-1: java.lang.IllegalStateException: Already connected
    at sun.net.www.protocol.http.HttpURLConnection.setRequestProperty(HttpURLConnection.java:2235)
    at sun.net.www.protocol.https.HttpsURLConnectionImpl.setRequestProperty(HttpsURLConnectionImpl.java:296)
    at org.apache.xmlrpc.client.XmlRpcSunHttpTransport.setRequestHeader(XmlRpcSunHttpTransport.java:73)
    at org.apache.xmlrpc.client.XmlRpcHttpTransport.setContentLength(XmlRpcHttpTransport.java:118)
    at org.apache.xmlrpc.client.XmlRpcHttpTransport.newReqWriter(XmlRpcHttpTransport.java:156)
    at org.apache.xmlrpc.client.XmlRpcStreamTransport.sendRequest(XmlRpcStreamTransport.java:150)
    at org.apache.xmlrpc.client.XmlRpcHttpTransport.sendRequest(XmlRpcHttpTransport.java:143)
    at org.apache.xmlrpc.client.XmlRpcSunHttpTransport.sendRequest(XmlRpcSunHttpTransport.java:69)
    at org.apache.xmlrpc.client.XmlRpcClientWorker.execute(XmlRpcClientWorker.java:56)
    at org.apache.xmlrpc.client.XmlRpcClient.execute(XmlRpcClient.java:167)
    at org.apache.xmlrpc.client.XmlRpcClient.execute(XmlRpcClient.java:137)
    at org.apache.xmlrpc.client.XmlRpcClient.execute(XmlRpcClient.java:126)
    at org.opensolaris.auth.client.AuthClient$XmlRpcInvocationHandler.invoke(AuthClient.java:258)
    at $Proxy7.encryptCookie(Unknown Source)
    at authclient.DL.run(Deadlock.java:84)
    at java.lang.Thread.run(Thread.java:619)
Thread-0: InputStreamTracer(sun.net.www.protocol.http.HttpURLConnection$HttpInputStream@1e13e07)
Thread-0: read() = "<"
Thread-0: read() = "?"
Thread-0: read() = "x"
Thread-0: read() = "m"
Thread-0: read(buff, 0, 28) = 28 "l version="1.0" encoding="UT"
Thread-0: read() = "F"
Thread-0: read() = "-"
Thread-0: read() = "8"
Thread-0: read() = """
Thread-0: read() = "?"
Thread-0: read() = ">"
Thread-0: read(buff, 0, 8192) = 949 "<methodResponse><params><param><value><struct><member><name>userId</name><value><i4>1</i4></value></member> ..."
Thread-0: read(buff, 0, 8192) = -1
Thread-0: close()
----------

Note that unlike the previous case, the reads from the socket are not intermixed, and Thread-0 sees the correct response.  Note however the following line and the subsequent stack trace:

Thread-1: java.lang.IllegalStateException: Already connected

Here's the issue: The underlying URLConnection has two states - opened and connected - see http://java.sun.com/javase/6/docs/api/java/net/URLConnection.html.  You can only set headers when the UrlConnection is in the opened state, once it becomes connected you can't modify headers.  When Thread-0 exits the synchronized sendRequest method, Thread-1 acquires the lock, but the shared UrlConnection is already in the connected state, so the attempt by Thread-1 to set headers fails.

Which takes us back to my original comment in the bug - XmlRpcClient just *isn't* thread-safe, despite what the docs say.  You may get away with using it in multi-threaded apps using a transport other than the SunHttpTransport , you may get away with running it on a uniprocessor machine, you may get away with only running it under light load, but using the Sun transport on a MP machine under heavy load it is just plain broken.

I'll carry on investigating and see if I can figure out a fix, but I suspect it is going to require either a rewrite or some aggressively coarse-grained locking.


> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Jochen Wiedmann (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707359#action_12707359 ] 

Jochen Wiedmann commented on XMLRPC-167:
----------------------------------------

Sure, it could, but what for?

By design, the documented anchor point for thread safety is the XmlRpcClient. Creating an XmlRpcTransport is a rather cheap operation.

Looking through your patch, I have no problem with explicitly disabling caches, or explicitly stating to use in- and output. If that makes the class currently thread safe, fine with me. But I'd never promise to keep it that way.

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707367#action_12707367 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

That's fine, that's exactly what I am suggesting.  Only the synchronisation of sendRequest is directly related to the race problem, the rest of the patch is just general cleanup and typo fixes.  I can move that stuff into another bug if you wish.

And I still think it is worth adding a note somewhere to the effect that the transport factories need to create a new transport for each request - the comment in XmlRpcClient misled me into believing that they didn't have to.  I can submit a patch for that too if you like.

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>         Attachments: XMLRPC-167.patch
>
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (XMLRPC-167) XmlRpcClient is supposed to be thread-safe but it isn't

Posted by "Alan Burlison (JIRA)" <xm...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/XMLRPC-167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12704376#action_12704376 ] 

Alan Burlison commented on XMLRPC-167:
--------------------------------------

I couldn't replicate the problem using those test classes even before the fix for XMLRPC-168, but my real app showed it within a few seconds.  I'll upgrade my XML-RPC libs to include the fix for XMLRPC-168 and start poking around tomorrow - I started looking at it today while you were very kindly looking at XMLRPC-168, but other things got in the way.  I'll let you know what I find.  Thanks!

> XmlRpcClient is supposed to be thread-safe but it isn't
> -------------------------------------------------------
>
>                 Key: XMLRPC-167
>                 URL: https://issues.apache.org/jira/browse/XMLRPC-167
>             Project: XML-RPC
>          Issue Type: Bug
>          Components: Source
>    Affects Versions: 3.1.2
>         Environment: Solaris
>            Reporter: Alan Burlison
>
> http://ws.apache.org/xmlrpc/apidocs/org/apache/xmlrpc/client/XmlRpcClient.html says:
> ----------
> A configured XmlRpcClient object is thread safe: In other words, the suggested use is, that you configure the client using setTransportFactory(XmlRpcTransportFactory) and similar methods, store it in a field and never modify it again. Without modifications, the client may be used for an arbitrary number of concurrent requests.
> ----------
> I have a simple test case that creates two threads that share a single XmlRpcClient instance, and if I run the test I see the following errors:
> ----------
> Fatal Error] :1:1: Content is not allowed in prolog.
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> [Fatal Error] :1:10: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlvrsion" must be followed by either attribute specifications, ">" or "/>".
> ----------
> ----------
> [Fatal Error] :1:1: Content is not allowed in prolog.
> [Fatal Error] :1:5: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Element type "xlv" must be followed by either attribute specifications, ">" or "/>".
> operation failed, Failed to parse server's response: Content is not allowed in prolog.
> ----------
> If I wrap the call to XmlRpcClient.execute with a synchronized block that locks on the XmlRpcClient object, the code works fine.
> It looks like the execute code is not locking around the read/write to/from the server, so the threads are competing to read the response from the server, and are tripping over each other.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.