[ https://issues.apache.org/jira/browse/DRILL-6004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16300341#comment-16300341 ]
ASF GitHub Bot commented on DRILL-6004: --------------------------------------- Github user vrozov commented on a diff in the pull request: https://github.com/apache/drill/pull/1070#discussion_r158335955 --- Diff: exec/memory/base/src/main/java/org/apache/drill/exec/memory/BoundsChecking.java --- @@ -17,19 +17,92 @@ */ package org.apache.drill.exec.memory; +import java.lang.reflect.Field; +import java.util.Formatter; + +import com.google.common.base.Preconditions; + +import io.netty.buffer.AbstractByteBuf; +import io.netty.buffer.DrillBuf; +import io.netty.util.IllegalReferenceCountException; + +import static org.apache.drill.exec.util.SystemPropertyUtil.getBoolean; + public class BoundsChecking { - static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BoundsChecking.class); + private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BoundsChecking.class); - public static final boolean BOUNDS_CHECKING_ENABLED; + public static final String ENABLE_UNSAFE_BOUNDS_CHECK_PROPERTY = "drill.exec.memory.enable_unsafe_bounds_check"; + // for backward compatibility check "drill.enable_unsafe_memory_access" property and enable bounds checking when + // unsafe memory access is explicitly disabled + public static final String ENABLE_UNSAFE_MEMORY_ACCESS_PROPERTY = "drill.enable_unsafe_memory_access"; + public static final boolean BOUNDS_CHECKING_ENABLED = + getBoolean(ENABLE_UNSAFE_BOUNDS_CHECK_PROPERTY, !getBoolean(ENABLE_UNSAFE_MEMORY_ACCESS_PROPERTY, true)); + private static final boolean checkAccessible = getStaticBooleanField(AbstractByteBuf.class, "checkAccessible", false); static { - boolean isAssertEnabled = false; - assert isAssertEnabled = true; - BOUNDS_CHECKING_ENABLED = isAssertEnabled - || !"true".equals(System.getProperty("drill.enable_unsafe_memory_access")); + if (BOUNDS_CHECKING_ENABLED) { + logger.warn("Drill is running with direct memory bounds checking enabled. If this is a production system, disable it."); + } else if (logger.isDebugEnabled()) { + logger.debug("Direct memory bounds checking is disabled."); + } } private BoundsChecking() { } + private static boolean getStaticBooleanField(Class cls, String name, boolean def) { + try { + Field field = cls.getDeclaredField(name); + field.setAccessible(true); + return field.getBoolean(null); + } catch (ReflectiveOperationException e) { + return def; + } + } + + private static void checkIndex(DrillBuf buf, int index, int fieldLength) { + Preconditions.checkNotNull(buf); + if (checkAccessible && buf.refCnt() == 0) { + Formatter formatter = new Formatter().format("%s, refCnt: 0", buf); + if (BaseAllocator.DEBUG) { + formatter.format("%n%s", buf.toVerboseString()); + } + throw new IllegalReferenceCountException(formatter.toString()); + } + if (fieldLength < 0) { + throw new IllegalArgumentException(String.format("length: %d (expected: >= 0)", fieldLength)); + } + if (index < 0 || index > buf.capacity() - fieldLength) { + Formatter formatter = new Formatter().format("%s, index: %d, length: %d (expected: range(0, %d))", buf, index, fieldLength, buf.capacity()); + if (BaseAllocator.DEBUG) { + formatter.format("%n%s", buf.toVerboseString()); + } + throw new IndexOutOfBoundsException(formatter.toString()); + } + } + + public static void lengthCheck(DrillBuf buf, int start, int length) { + if (BOUNDS_CHECKING_ENABLED) { + checkIndex(buf, start, length); + } + } + + public static void rangeCheck(DrillBuf buf, int start, int end) { + if (BOUNDS_CHECKING_ENABLED) { + checkIndex(buf, start, end - start); + } + } + + public static void rangeCheck(DrillBuf buf1, int start1, int end1, DrillBuf buf2, int start2, int end2) { --- End diff -- It is a common case for `equal()` and `compare()`. > Direct buffer bounds checking should be disabled by default > ----------------------------------------------------------- > > Key: DRILL-6004 > URL: https://issues.apache.org/jira/browse/DRILL-6004 > Project: Apache Drill > Issue Type: Improvement > Reporter: Vlad Rozov > Assignee: Vlad Rozov > Priority: Minor > > Direct buffer bounds checking is enabled either when assertions are enabled > (see DRILL-6001) or when {{drill.enable_unsafe_memory_access}} property is > not set to true, so it is enabled in production as by default > {{drill.enable_unsafe_memory_access}} is not set. -- This message was sent by Atlassian JIRA (v6.4.14#64029)