Skip to content

Conversation

@richox
Copy link
Contributor

@richox richox commented Dec 31, 2025

Which issue does this PR close?

Closes #1803

Rationale for this change

What changes are included in this PR?

Fixes possible synchronize block confliction in OnHeapSpillManager

Are there any user-facing changes?

No.

How was this patch tested?

Rerun the online buggy query many times to verify that the bug has gone.

@github-actions github-actions bot added the spark label Dec 31, 2025
@richox richox changed the title fix possible deadlock in OnHeapSpillManager [AURON #1803] fix possible deadlock in OnHeapSpillManager Dec 31, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a potential deadlock in the OnHeapSpillManager by improving synchronization between OnHeapSpill and SparkOnHeapSpillManager. The deadlock could occur when an OnHeapSpill.write() operation holds a lock while calling acquireMemory(), which in turn triggers SparkOnHeapSpillManager.spill() that attempts to acquire the same lock again.

Key Changes:

  • Replaced intrinsic synchronized locks with explicit ReentrantLock for better control over lock acquisition
  • Added a trigger parameter to the spill() method to identify the initiating MemoryConsumer
  • Implemented deadlock prevention logic using tryLock() when the spilling is triggered by the same SparkOnHeapSpillManager instance

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
OnHeapSpill.scala Replaces synchronized blocks with ReentrantLock, adds trigger parameter to spill() method with tryLock logic to prevent deadlock when spilling is triggered by the same thread
SparkOnHeapSpillManager.scala Updates call to spill.spill() to pass the trigger parameter that identifies which MemoryConsumer initiated the spilling operation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OnHeapSpillManager unexpectly runs into a deadlock

3 participants