[
https://issues.apache.org/jira/browse/THRIFT-845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900124#action_12900124
]
Bryan Duxbury commented on THRIFT-845:
--------------------------------------
bq. Only the select thread modifies the timeoutWatchSet - methods are entered
when they are popped off the pending queue, not when they're pushed on.
Having threads involved is only one way to get a ConcurrentModification
exception. Try this code out:
{code}
import java.util.HashSet;
public class QuickTest {
public static void main(String[] args) {
HashSet<String> set = new HashSet<String>();
set.add("blah");
set.add("blah2");
for (String s : set) {
set.remove("blah2");
}
}
}
{code}
If you try to remove any value from the set that is actually in the set,
whether it is the current element being iterated or not, causes a CME. Trying
to remove an element not in the set will not throw any exception. And if the
item you are removing is the *only* item in the set, then you also won't get an
exception.
> async client does not respect timeout
> -------------------------------------
>
> Key: THRIFT-845
> URL: https://issues.apache.org/jira/browse/THRIFT-845
> Project: Thrift
> Issue Type: Bug
> Components: Library (Java)
> Affects Versions: 0.4, 0.5
> Reporter: Eric Jensen
> Fix For: 0.5
>
> Attachments: async_thrift.diff
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> there effectively is no timeout on calls via the async client, so the
> consumer may never be called back with either a success or error. this is
> not an awesome contract, and it also leaves the potential for the client
> itself to leak. ning is working on a patch for this based on the below
> discussion:
> implementing the timeout inside TAsyncClientManager seems pretty easy, we
> would just loop through Selector.keys() there and make sure none of them have
> been waiting for longer than the SO_TIMEOUT we set on its TNonblockingSocket,
> if one is, we just do methodCall.onError on it to make sure the caller is
> notified.
> We also need to change Selector.select() to Selector.select(long timeout),
> with a small value so that the selector thread wakes up with sufficient
> granularity to timeout calls.
> I think we can just track the timeout of the pending operation within
> TAsyncMethodCall itself and loop over Selector.keys() to check them. But
> since we just made the change to leave the keys for all open connections
> registered with the selector until they are closed, this could be somewhat
> costly.
> For reference, here's how jetty does this
> (http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-io/8.0.0.M0/org/eclipse/jetty/io/nio/SelectorManager.java#SelectorManager)
> which i do _not_ encourage you to read, but i am confident from reading it
> that it's doing just what i said: tracking the list of outstanding
> read/write operations and their timeouts themselves, looping through them
> within the selector thread, and closing their sockets if their so-called
> "idle timeout" has expired.
> Also, there are a couple java selector bugs jetty links to that we should be
> aware of, the only one applicable to us has been fixed in jdk 1.6 update 18
> (the latest is 21).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
