Hello
I have code like this:TNonblockingTransport transport = new
TNonblockingSocket("127.0.0.1", 9160, 10000);TAsyncClientManager clientManager
= new TAsyncClientManager();TProtocolFactory protocolFactory = new
TBinaryProtocol.Factory();
//generated thrift clientZipkinCollector.AsyncClient c = new
ZipkinCollector.AsyncClient( protocolFactory, clientManager,
transport);//calling methods in other threadsc.Log....
I am using async client with nio support (libthrift 0.9.1). I was looking
inside the code and I was surprised about ___currentMethod in
https://github.com/apache/thrift/blob/0.9.1/lib/java/src/org/apache/thrift/async/TAsyncClient.java#L28
My first question is the purpose of this variable.
If it is a check to serialize reads and writes to NIO socket than you have a
bug there. In this case you have unprotected critical section on
___currentMethod. It can be mutated from two threads - one is Selector thread
in TAsyncClientManager and the second one is main thread calling generated
thrift protocol method (in this case Log)
public void Log(List<LogEntry> messages,
org.apache.thrift.async.AsyncMethodCallback resultHandler) throws
org.apache.thrift.TException { checkReady(); Log_call method_call = new
Log_call(messages, resultHandler, this, ___protocolFactory, ___transport);
this.___currentMethod = method_call; //<------mutating
___manager.call(method_call);}
This can lead to unpredictable results.
Can some maintainer confirm this ? I would fill you a bug but at first I want
to be sure that I did not missed something crucial.
Either way you should really reconsider using mutable variables in asynchronous
environment. This can lead to serious mess.
Regards
Peter CipovJánošík
