[ https://issues.apache.org/jira/browse/HADOOP-15859?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16654074#comment-16654074 ]
Kihwal Lee commented on HADOOP-15859: ------------------------------------- +1 We've applied the same patch internally. > 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 > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org