[jira] [Commented] (HDFS-11976) Examine code base for cases that exception is thrown from finally block and fix it

2017-06-15 Thread Pavel Avgustinov (JIRA)

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

Pavel Avgustinov commented on HDFS-11976:
-

This concern is easy to capture with the query language used by 
[lgtm|https://lgtm.com]. I've just written a query for it, which you can see 
[here|https://lgtm.com/query/1995330256/project:5629499534213120/lang:java/] -- 
it finds 32 results in the [GitHub mirror of 
Hadoop|https://github.com/apache/hadoop], including a number within HDFS itself.

(Disclaimer: I'm a co-founder of Semmle, the company that provides lgtm as a 
free service to open source.)

> Examine code base for cases that exception is thrown from finally block and 
> fix it
> --
>
> Key: HDFS-11976
> URL: https://issues.apache.org/jira/browse/HDFS-11976
> Project: Hadoop HDFS
>  Issue Type: Bug
>Reporter: Yongjun Zhang
>
> If exception X is thrown in try block, and exception Y is thrown is finally 
> block, X will be swallowed.
> In addition, finally block is used to ensure resources are released properly 
> in general. If we throw exception from there, some resources may be leaked. 
> So it's not recommended to throw exception in the finally block
> I caught one today and reported HDFS-11794, creating this jira as a master 
> one to catch other similar cases.
> Hope there is some static analyzer to find all.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org



[jira] [Updated] (HDFS-10183) Prevent race condition during class initialization

2016-03-20 Thread Pavel Avgustinov (JIRA)

 [ 
https://issues.apache.org/jira/browse/HDFS-10183?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Pavel Avgustinov updated HDFS-10183:

Attachment: HDFS-10183.2.patch

Adding v2 of the patch, which resolves the new checkstyle warnings by renaming 
the newly {{final}} fields to uppercase. I'm a bit mystified by the test 
failures, and for me locally the test suite doesn't pass with or without the 
patch. I'll try to diagnose, but if the answer is obvious to anyone I'd 
appreciate a heads-up.

[~sjlee0], good point about 
[JLS12.4.2|https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.2].
 It indeed suggests that something slightly more subtle is going on, as the 
situation I described in the original issue would not lead to a data race -- 
the acquisition of the class loading lock would order the write to the field in 
whichever thread wins the initialization race and the read in the other 
thread(s).

I suspect the answer is actually in the previous section 
([JLS12.4.1|https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.1]),
 which describes when class loading happens (i.e. when the procedure in 12.4.2 
applies). In particular, it says "A class or interface type T will be 
initialized immediately before the first occurrence of \[an instance creation 
or static member access\]". In other words, if the JVM can tell that a class 
has already been initialized, it does not need to perform the procedure of 
12.4.2.

With this in mind, a potential explanation for the problem is this: One thread 
initializes the class with the non-{{final}} {{ThreadLocal}} field, and another 
tries to use it shortly after, but without triggering its own initialization. 
Because it does not acquire the class loading lock, there is no happens-before 
relationship between its read of the static field and the write that happened 
during class initialization, and if we're unlucky the write has not in fact 
propagated.

I tried to come up with a small example demonstrating the issue, but couldn't 
trigger it. I speculate that it needs a certain critical mass of complexity in 
order to make the HotSpot compiler kick in and apply the necessary 
optimizations. The stack trace shown by [~busbey] in HADOOP-11969 is a clear 
example of it being a problem in practice, though -- in the state of the tree 
before that fix, the only write to the {{DECODER_FACTORY}} field is the field 
initializer (which sets it to non-{{null}}), and so the exception observed 
would be impossible without a data race along the lines described above.

> Prevent race condition during class initialization
> --
>
> Key: HDFS-10183
> URL: https://issues.apache.org/jira/browse/HDFS-10183
> Project: Hadoop HDFS
>  Issue Type: Bug
>  Components: fs
>Affects Versions: 2.9.0
>Reporter: Pavel Avgustinov
>Assignee: Pavel Avgustinov
>Priority: Minor
> Fix For: 2.9.0
>
> Attachments: HADOOP-12944.1.patch, HDFS-10183.2.patch
>
>
> In HADOOP-11969, [~busbey] tracked down a non-deterministic 
> {{NullPointerException}} to an oddity in the Java memory model: When multiple 
> threads trigger the loading of a class at the same time, one of them wins and 
> creates the {{java.lang.Class}} instance; the others block during this 
> initialization, but once it is complete they may obtain a reference to the 
> {{Class}} which has non-{{final}} fields still containing their default (i.e. 
> {{null}}) values. This leads to runtime failures that are hard to debug or 
> diagnose.
> HADOOP-11969 observed that {{ThreadLocal}} fields, by their very nature, are 
> very likely to be accessed from multiple threads, and thus the problem is 
> particularly severe there. Consequently, the patch removed all occurrences of 
> the issue in the code base.
> Unfortunately, since then HDFS-7964 has [reverted one of the fixes during a 
> refactoring|https://github.com/apache/hadoop/commit/2151716832ad14932dd65b1a4e47e64d8d6cd767#diff-0c2e9f7f9e685f38d1a11373b627cfa6R151],
>  and introduced a [new instance of the 
> problem|https://github.com/apache/hadoop/commit/2151716832ad14932dd65b1a4e47e64d8d6cd767#diff-6334d0df7d9aefbccd12b21bb7603169R43].
> The attached patch addresses the issue by adding the missing {{final}} 
> modifier in these two cases.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (HDFS-10183) Prevent race condition during class initialization

2016-03-19 Thread Pavel Avgustinov (JIRA)

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

Pavel Avgustinov commented on HDFS-10183:
-

[~daryn] -- no concrete issues, though of course they'd be hard to trigger.

> Prevent race condition during class initialization
> --
>
> Key: HDFS-10183
> URL: https://issues.apache.org/jira/browse/HDFS-10183
> Project: Hadoop HDFS
>  Issue Type: Bug
>  Components: fs
>Affects Versions: 2.9.0
>Reporter: Pavel Avgustinov
>Assignee: Pavel Avgustinov
>Priority: Minor
> Fix For: 2.9.0
>
> Attachments: HADOOP-12944.1.patch
>
>
> In HADOOP-11969, [~busbey] tracked down a non-deterministic 
> {{NullPointerException}} to an oddity in the Java memory model: When multiple 
> threads trigger the loading of a class at the same time, one of them wins and 
> creates the {{java.lang.Class}} instance; the others block during this 
> initialization, but once it is complete they may obtain a reference to the 
> {{Class}} which has non-{{final}} fields still containing their default (i.e. 
> {{null}}) values. This leads to runtime failures that are hard to debug or 
> diagnose.
> HADOOP-11969 observed that {{ThreadLocal}} fields, by their very nature, are 
> very likely to be accessed from multiple threads, and thus the problem is 
> particularly severe there. Consequently, the patch removed all occurrences of 
> the issue in the code base.
> Unfortunately, since then HDFS-7964 has [reverted one of the fixes during a 
> refactoring|https://github.com/apache/hadoop/commit/2151716832ad14932dd65b1a4e47e64d8d6cd767#diff-0c2e9f7f9e685f38d1a11373b627cfa6R151],
>  and introduced a [new instance of the 
> problem|https://github.com/apache/hadoop/commit/2151716832ad14932dd65b1a4e47e64d8d6cd767#diff-6334d0df7d9aefbccd12b21bb7603169R43].
> The attached patch addresses the issue by adding the missing {{final}} 
> modifier in these two cases.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)