Skip to content

Commit

Permalink
Fix regression HostColumnVectorCore requiring native libs (#9948)
Browse files Browse the repository at this point in the history
#9485 regressed the fix in #9332.  This restores the fix.  It also eliminates the problematic `ColumnVector.closeBuffers` which, despite the plurality in the name, only closed one buffer and had dubious utility in converting throwables into runtime exceptions.

Authors:
  - Jason Lowe (https://github.com/jlowe)

Approvers:
  - Alessandro Bellina (https://github.com/abellina)
  - Robert (Bobby) Evans (https://github.com/revans2)
  - Gera Shegalov (https://github.com/gerashegalov)

URL: #9948
  • Loading branch information
jlowe authored Jan 5, 2022
1 parent eba4f03 commit 33f7f0d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 28 deletions.
36 changes: 11 additions & 25 deletions java/src/main/java/ai/rapids/cudf/ColumnVector.java
Original file line number Diff line number Diff line change
Expand Up @@ -907,24 +907,6 @@ private static native long stringConcatenationSepCol(long[] columnViews,
// INTERNAL/NATIVE ACCESS
/////////////////////////////////////////////////////////////////////////////

/**
* Close all non-null buffers. Exceptions that occur during the process will
* be aggregated into a single exception thrown at the end.
*/
static void closeBuffers(AutoCloseable buffer) {
Throwable toThrow = null;
if (buffer != null) {
try {
buffer.close();
} catch (Throwable t) {
toThrow = t;
}
}
if (toThrow != null) {
throw new RuntimeException(toThrow);
}
}

////////
// Native methods specific to cudf::column. These either take or create a cudf::column
// instead of a cudf::column_view so they need to be used with caution. These should
Expand Down Expand Up @@ -1114,13 +1096,17 @@ protected synchronized boolean cleanImpl(boolean logErrorIfNotClean) {
if (!toClose.isEmpty()) {
try {
for (MemoryBuffer toCloseBuff : toClose) {
closeBuffers(toCloseBuff);
}
} catch (Throwable t) {
if (toThrow != null) {
toThrow.addSuppressed(t);
} else {
toThrow = t;
if (toCloseBuff != null) {
try {
toCloseBuff.close();
} catch (Throwable t) {
if (toThrow != null) {
toThrow.addSuppressed(t);
} else {
toThrow = t;
}
}
}
}
} finally {
toClose.clear();
Expand Down
12 changes: 9 additions & 3 deletions java/src/main/java/ai/rapids/cudf/HostColumnVectorCore.java
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,15 @@ protected synchronized boolean cleanImpl(boolean logErrorIfNotClean) {
boolean neededCleanup = false;
if (data != null || valid != null || offsets != null) {
try {
ColumnVector.closeBuffers(data);
ColumnVector.closeBuffers(offsets);
ColumnVector.closeBuffers(valid);
if (data != null) {
data.close();
}
if (offsets != null) {
offsets.close();
}
if (valid != null) {
valid.close();
}
} finally {
// Always mark the resource as freed even if an exception is thrown.
// We cannot know how far it progressed before the exception, and
Expand Down

0 comments on commit 33f7f0d

Please sign in to comment.