[SYCL][HIP] Fix crash from SYCL buffer destructors during shutdown#22679
[SYCL][HIP] Fix crash from SYCL buffer destructors during shutdown#22679zjin-lcf wants to merge 1 commit into
Conversation
Static SYCL buffer destructors run after the runtime has begun shutting down (their atexit handlers are registered before the runtime's). On HIP this crashed twice: waiting on events whose stream/context was already released segfaults inside libamdhip64, and releasing those events returns hipErrorContextIsDestroyed which was thrown out of the event_impl destructor. - Scheduler::removeMemoryObject: extend the existing Windows-only "skip wait during shutdown" guard to all platforms. Host memory is not written back during shutdown anyway, so the wait serves no purpose and is unsafe against partially torn-down adapters. - HIP adapter event release(): treat hipErrorContextIsDestroyed and hipErrorDeinitialized as a successful release instead of surfacing an exception. Re-enables Regression/static-buffer-dtor.cpp on HIP. Co-authored-by: Cursor <cursoragent@cursor.com>
| // (e.g. the HIP runtime segfaults when synchronizing on an event whose | ||
| // stream/context has been released). During shutdown host memory is not | ||
| // written back either (see SYCLMemObjT::updateHostMemory), so waiting serves | ||
| // no purpose here and the memory WILL be reclaimed as the process exits. |
There was a problem hiding this comment.
AFAIK on Linux we were able to correctly shutdown SYCL RT and release its resources. This change disables proper resource release on Linux (on Windows due to thread's management it was disabled by default).
Could you please clarify what exact resources are released in unexpected order? And callstack where it happens.
There was a problem hiding this comment.
@cperkinsintel could you please provide your opinion about this change? I found that this test was disabled for HIP ~5 years ago, I assume that there are some static vars in HIP runtime that have a conflict with SYCL RT internals in terms of order. It seems to be a HIP specific issue. I am pretty hesitant to disable proper shutdown cleanup on Linux because of that.
2 participants
Summary
Regression/static-buffer-dtoron HIP.Test plan
Regression/static-buffer-dtorpasses on gfx942 (repeated runs)Relates to #22300