Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Mon, 22 Jul 2013 15:48:49 +0000 (UTC)
From: "Mikhail Mazursky (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.12654369.1371974097259.90816.1374508129912@arcas>
In-Reply-To: <JIRA.12654369.1371974097259@arcas>
References: <JIRA.12654369.1371974097259@arcas>
Subject: [jira] [Commented] (CASSANDRA-5690) Fix AsyncOneResponse
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/CASSANDRA-5690?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13715309#comment-13715309 ] 

Mikhail Mazursky commented on CASSANDRA-5690:
---------------------------------------------

Pros for using "this":
- smaller object's memory footprint;

Cons:
- exposes synchronization i.e. breaks incapsulation of synchronization mechanics. This can possibly result in unexpected problems/interference if some other code tries to use this instance as it's own synchronization object - synchronized (AsyncOneResponse instance). See [1] for a bit more details.

IMO in this case it's better to hide synchronization details for safety reasons. But I can change patch if you think otherwise.

"done" doesn't need to be volatile because all access to this field is synchronized using lock. So this lock guarantees both mutual exclusion of concurrent response() method calls (prevents race on setting "result" and "done") and visibility by establishing happens-before between lock release (finish of response() method) and subsequent lock acquire (start of get() method and exit from corresponding Object.wait()). From [2]:

{quote}
Synchronization is built around an internal entity known as the intrinsic lock or monitor lock. (The API specification often refers to this entity simply as a "monitor.") Intrinsic locks play a role in both aspects of synchronization: enforcing exclusive access to an object's state and establishing happens-before relationships that are essential to visibility.

Every object has an intrinsic lock associated with it. By convention, a thread that needs exclusive and consistent access to an object's fields has to acquire the object's intrinsic lock before accessing them, and then release the intrinsic lock when it's done with them. A thread is said to own the intrinsic lock between the time it has acquired the lock and released the lock. As long as a thread owns an intrinsic lock, no other thread can acquire the same lock. The other thread will block when it attempts to acquire the lock.

When a thread releases an intrinsic lock, a happens-before relationship is established between that action and any subsequent acquistion of the same lock.
{quote}

For a similar code and more details see [3]. You may also want to read the famous great book Java Concurrency in Practice [4].

[1]: http://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java
[2]: http://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html
[3]: http://docs.oracle.com/javase/tutorial/essential/concurrency/guardmeth.html
[4]: http://www.amazon.com/exec/obidos/ASIN/0321349601
                
> Fix AsyncOneResponse
> --------------------
>
>                 Key: CASSANDRA-5690
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-5690
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Mikhail Mazursky
>            Assignee: Mikhail Mazursky
>            Priority: Minor
>         Attachments: trunk-5690.patch
>
>
> Current implementation of AsyncOneResponse suffers from two problems:
> 1. Spurious wakeup will lead to TimeoutException being thrown because awaiting for condition is not done in loop;
> 2. condition.signal() is used where .signalAll() should be used - this leads to only one thread blocked on .get() to be unblocked. Other threads will stay blocked forever.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira