Skip to content

chore: Register task completion listener to ensure CometExecIterator is always closed#3959

Open
wForget wants to merge 3 commits intoapache:mainfrom
wForget:minor
Open

chore: Register task completion listener to ensure CometExecIterator is always closed#3959
wForget wants to merge 3 commits intoapache:mainfrom
wForget:minor

Conversation

@wForget
Copy link
Copy Markdown
Member

@wForget wForget commented Apr 16, 2026

Which issue does this PR close?

Closes #.

Rationale for this change

CometExecIterator.close is not always registered to the task completion listener, which may lead to leaks when task fails. Like:

What changes are included in this PR?

Always register a task completion listener for CometExecIterator.close.

How are these changes tested?

private var closed: Boolean = false

// Register a task completion listener to ensure native resources are released when the task is done.
TaskContext.get().addTaskCompletionListener[Unit] { _ =>
Copy link
Copy Markdown
Member Author

@wForget wForget Apr 16, 2026

Choose a reason for hiding this comment

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

Since TaskContext.get() has already been called, the new behavior should be safe.

private val taskAttemptId = TaskContext.get().taskAttemptId
private val taskCPUs = TaskContext.get().cpus()

Copy link
Copy Markdown
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @wForget

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants