hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Lowe (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HADOOP-15859) ZStandardDecompressor.c mistakes a class for an instance
Date Wed, 17 Oct 2018 19:49:00 GMT

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

Jason Lowe updated HADOOP-15859:
--------------------------------
       Resolution: Fixed
     Hadoop Flags: Reviewed
    Fix Version/s: 3.1.2
                   3.0.4
                   2.9.2
                   3.2.0
                   2.10.0
           Status: Resolved  (was: Patch Available)

Thanks for the review, Kihwal!  I committed this to trunk, branch-3.2, branch-3.1, branch-3.0,
branch-2, and branch-2.9.

> ZStandardDecompressor.c mistakes a class for an instance
> --------------------------------------------------------
>
>                 Key: HADOOP-15859
>                 URL: https://issues.apache.org/jira/browse/HADOOP-15859
>             Project: Hadoop Common
>          Issue Type: Bug
>    Affects Versions: 2.9.0, 3.0.0-alpha2
>            Reporter: Ben Lau
>            Assignee: Jason Lowe
>            Priority: Blocker
>             Fix For: 2.10.0, 3.2.0, 2.9.2, 3.0.4, 3.1.2
>
>         Attachments: HADOOP-15859.001.patch
>
>
> As a follow up to HADOOP-15820, I was doing more testing on ZSTD compression and still
encountered segfaults in the JVM in HBase after that fix. 
> I took a deeper look and realized there is still another bug, which looks like it's that
we are actually [calling setInt()|https://github.com/apache/hadoop/blob/f13e231025333ebf80b30bbdce1296cef554943b/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/zstd/ZStandardDecompressor.c#L148]
on the "remaining" variable on the ZStandardDecompressor class itself (instead of an instance
of that class) because the Java stub for the native C init() function [is marked static|https://github.com/apache/hadoop/blob/a0a276162147e843a5a4e028abdca5b66f5118da/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/zstd/ZStandardDecompressor.java#L253],
leading to memory corruption and a crash during GC later.
> Initially I thought we would fix this by changing the Java init() method to be non-static,
but it looks like the "remaining" setInt() call is actually unnecessary anyway, because in
ZStandardDecompressor.java's reset() we [set "remaining" to 0 right after calling the JNI
init() call|https://github.com/apache/hadoop/blob/a0a276162147e843a5a4e028abdca5b66f5118da/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/zstd/ZStandardDecompressor.java#L216].
So ZStandardDecompressor.java init() doesn't have to be changed to an instance method, we
can leave it as static, but remove the JNI init() call's "remaining" setInt() call altogether.
> Furthermore we should probably clean up the class/instance distinction in the C file
because that's what led to this confusion. There are some other methods where the distinction
is incorrect or ambiguous, we should fix them to prevent this from happening again.
> I talked to [~jlowe] who further pointed out the ZStandardCompressor also has similar
problems and needs to be fixed too.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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


Mime
View raw message