Mailing-List: contact common-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Tue, 27 Jun 2017 11:55:00 +0000 (UTC)
From: "Uwe Schindler (JIRA)" <jira@apache.org>
To: common-issues@hadoop.apache.org
Message-ID: <JIRA.13082308.1498309092000.114556.1498564500208@Atlassian.JIRA>
In-Reply-To: <JIRA.13082308.1498309092000@Atlassian.JIRA>
References: <JIRA.13082308.1498309092000@Atlassian.JIRA> <JIRA.13082308.1498309092120@jira-lw-us.apache.org>
Subject: [jira] [Comment Edited] (HADOOP-14586)
  org.apache.hadoop.util.Shell in 2.7 breaks <clinit> on Java 9 RC build;
 backport HADOOP-10775 to 2.7.x
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Tue, 27 Jun 2017 11:55:05 -0000


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

Uwe Schindler edited comment on HADOOP-14586 at 6/27/17 11:54 AM:
------------------------------------------------------------------

Hi Steve,

bq. I want the patch which selectively cherry picks the minimum bits of the 2.8 code, particular that const and the IsJava7OrAbove() function call. We could get away with the other obsolete cruft.

I would go minimal here. Removeing a lot of obsolete cruft is to risky in 2.7 (that's my opinion). If the constant is static and final, the optimizer will remove the cruft anyways :-)

Nevertheless, I am fine with both ideas! My latest patch is just a small change without any actual code change. It just changes the constant value to hardcoded "true" instead of the failing property check that will always return "true" based on the Hadoop 2.7 minimum Java requirements. To me this is just a "bugfix" on the constant definition in Hadoop 2.7, no longer applicable to later versions.

bq. Why? I don't want 2.7.x to actually add new code which isn't a backport

That's the reason why I submitted my patch. It does not change any code/logic, it just replaces a function call by a constant value, which is constant anyways.

I agree that the whole issue only affects Hadoop 2.7 and before, so I am sorry for initially arguing that also Hadoop 2.8 or 3.0 has the same problem. I should have checked the code, but I was not aware of any changes. As I am not a Hadoop developer, I did not checkout any code, I just looked at the source code as returned by my IDE fom Maven Central. Sorry for getting aggressive, but I hope you agree that code like we have seen here should have never been in Hadoop!

Finally I just repeat: This is issue is not dircetly related to Java 9, the attached commit just fixes "buggy code": It misses to add bounds checks when parsing a string that is not controlled by Hadoop - coming from the outside. This may be fixed in Hadoop 2.8, but 2.7 is still under support and the commit logs show that a lot of stuff is committed to the 2.7 branch.


was (Author: thetaphi):
Hi Steve,

bq. I want the patch which selectively cherry picks the minimum bits of the 2.8 code, particular that const and the IsJava7OrAbove() function call. We could get away with the other obsolete cruft.

I would go minimal here. Removeing a lot of obsolete cruft is to risky in 2.7 (that's my opinion). If the constant is static and final, the optimizer will remove the cruft anyways :-)

Nevertheless, I am fine with both ideas! My latest patch is just a small change without any actual code change. It just changes the constant value to hardcoded "true" instead of the failing property check that will always return "true" based on the Hadoop 2.7 minimum Java requirements. To me this is just a "bugfix" on the constant definition in Hadoop 2.7, no longer applicable to later versions.

bq. Why? I don't want 2.7.x to actually add new code which isn't a backport

That's the reason why I submitted my patch. It does not change any code/logic, it just replaces a function call by a constant value, which is constant anyways.

I agree that the whole issue only affects Hadoop 2.7, so I am sorry for initially arguing that also Hadoop 2.8 or 3.0 has the same problem. I should have checked the code, but I was not aware of any changes. As I am not a Hadoop developer, I did not checkout any code, I just looked at the source code as returned by my IDE fom Maven Central. Sorry for getting aggressive, but I hope you agree that code like we have seen here should have never been in Hadoop!

Finally I just repeat: This is issue is not dircetly related to Java 9, the attached commit just fixes "buggy code": It misses to add bounds checks when parsing a string that is not controlled by Hadoop - coming from the outside. This may be fixed in Hadoop 2.8, but 2.7 is still under support and the commit logs show that a lot of stuff is committed to the 2.7 branch.

>  org.apache.hadoop.util.Shell in 2.7 breaks <clinit> on Java 9 RC build; backport HADOOP-10775 to 2.7.x
> -------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-14586
>                 URL: https://issues.apache.org/jira/browse/HADOOP-14586
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: common
>    Affects Versions: 2.7.2
>         Environment: Java 9, build 175 (Java 9 release candidate as of June 25th, 2017)
>            Reporter: Uwe Schindler
>            Assignee: Akira Ajisaka
>            Priority: Blocker
>              Labels: Java9, release-blocker
>         Attachments: HADOOP-14586-branch-2.7-01.patch, HADOOP-14586-branch-2.7-02.patch
>
>
> You cannot use any Hadoop component anymore with the latest release candidate build of Java 9, because it fails with an StringIndexOutOfBoundsException in {{org.apache.hadoop.util.Shell#<clinit>}}. This leads to a whole cascade of failing classes (next in chain is StringUtils).
> The reason is that the release candidate build of Java 9 no longer has "-ea" in the version string and the system property "java.version" is now simply "9". This causes the following line to fail fatally:
> {code:java}
>   private static boolean IS_JAVA7_OR_ABOVE =
>       System.getProperty("java.version").substring(0, 3).compareTo("1.7") >= 0;
> {code}
> Analysis:
> - This code looks wrong, as comparing a version this way is incorrect.
> - The {{substring(0, 3)}} is completely useless, {{compareTo}} also works without it, although it is still an invalid way to compare a version.
> Maybe look at Lucene's source code (Constants.java) to have a better way in doing this! Sorry this is incredible to me! Hardcoding string bounds into a static initializer that are applied on a string that you have no control of...
> This issue breaks Apache Solr from working with Java 9, so I set it to "critical". We have to disable the whole Hadoop integration once Java 9 is detected.


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

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org