Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression HostColumnVectorCore requiring native libs #9948

Merged
merged 1 commit into from
Jan 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -872,24 +872,6 @@ private static native long hash(long[] viewHandles, int hashId, int[] initialVal
// 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 @@ -1079,13 +1061,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();
}
Comment on lines +597 to +605
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could benefit from some 3rd-party or our own flavor of API callable like this: IOUtils.close(data, offsets, valid);

} 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