[
https://issues.apache.org/jira/browse/HADOOP-3455?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12600640#action_12600640
]
Steve Loughran commented on HADOOP-3455:
----------------------------------------
If you dont mark a field as volatile
1. it can be cached forever, so something going while(!ready) { pause } could
spin forever.
2. accesses can be reordered. In fact, Java 1.4 and below can reorder stuff
marked volatile too, just to punish people trying to be clever
see : http://www.cs.umd.edu/~pugh/java/memoryModel/index.html#reference
If you want to share data across threads, you need to follow Tom's rules:
synchronized blocks or volatile. If you want to do any atomic set/update
operations, synchronized only or the java5 java.utils.concurrent.atomic types,
which are very efficient for their operations, but fiddly to work with.
> IPC.Client synchronisation looks weak
> -------------------------------------
>
> Key: HADOOP-3455
> URL: https://issues.apache.org/jira/browse/HADOOP-3455
> Project: Hadoop Core
> Issue Type: Improvement
> Components: ipc
> Affects Versions: 0.18.0
> Reporter: Steve Loughran
> Attachments: hadoop-3455.patch
>
>
> Looking at HADOOP-3453 , its clear that Client.java is inconsistently
> synchronized
> 1. the running and shouldCloseConnection flags are not always read/written in
> synchronized blocks, even though they are properties used to share
> information between threads. They should be marked as volatile for access
> outside synchronized blocks, and all read-check-update operations must be
> synchronized.
> 2. there are multiple calls to System.currentTimeMillis() in synchronized
> blocks; this is a slow native operation and should ideally be done
> unsynchronized.
> 3. Synchronizing on the (out) stream is dangerous as its value changes during
> the life of the class, and sometimes it is null. These blocks should all
> synchronize on the Client instead.
> 4. There are a number of places where InterruptedExceptions are caught and
> ignored in a sleep-wait loop:
> } catch (InterruptedException e) {
> }
> This isn't dangerous, but it does make the client harder to stop. These
> code fragments should be looked at carefully.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
