cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mikhail Mazursky (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-5690) Fix AsyncOneResponse
Date Mon, 22 Jul 2013 15:48:49 GMT

    [ 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

Mime
View raw message