[SYCL][L0] Fix EventPool memory leak (#4080)

We need to destroy event pools at the tear-down time regardless of its any live events.

Signed-off-by: Byoungro So <[email protected]>

Author: bso-intel

sycl/plugins/level_zero/pi_level_zero.cpp

@@ -335,15 +335,16 @@ _pi_context::getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &ZePool,
   if ((ZeEventPool == nullptr) ||
       (NumEventsAvailableInEventPool[ZeEventPool] == 0)) {
     // Creation of the new ZePool with record in NumEventsAvailableInEventPool
-    // and initialization of the record in NumEventsLiveInEventPool must be done
-    // atomically. Otherwise it is possible that decrementAliveEventsInPool will
-    // be called for the record in NumEventsLiveInEventPool before its
+    // and initialization of the record in NumEventsUnreleasedInEventPool must
+    // be done atomically. Otherwise it is possible that
+    // decrementUnreleasedEventsInPool will be called for the record in
+    // NumEventsUnreleasedInEventPool before its
     std::lock(NumEventsAvailableInEventPoolMutex,
-              NumEventsLiveInEventPoolMutex);
+              NumEventsUnreleasedInEventPoolMutex);
     std::lock_guard<std::mutex> NumEventsAvailableInEventPoolGuard(
         NumEventsAvailableInEventPoolMutex, std::adopt_lock);
-    std::lock_guard<std::mutex> NumEventsLiveInEventPoolGuard(
-        NumEventsLiveInEventPoolMutex, std::adopt_lock);
+    std::lock_guard<std::mutex> NumEventsUnreleasedInEventPoolGuard(
+        NumEventsUnreleasedInEventPoolMutex, std::adopt_lock);
 
     ZeStruct<ze_event_pool_desc_t> ZeEventPoolDesc;
     ZeEventPoolDesc.count = MaxNumEventsPerPool;
@@ -362,7 +363,7 @@ _pi_context::getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &ZePool,
     ZE_CALL(zeEventPoolCreate, (ZeContext, &ZeEventPoolDesc, ZeDevices.size(),
                                 &ZeDevices[0], &ZeEventPool));
     NumEventsAvailableInEventPool[ZeEventPool] = MaxNumEventsPerPool - 1;
-    NumEventsLiveInEventPool[ZeEventPool] = MaxNumEventsPerPool;
+    NumEventsUnreleasedInEventPool[ZeEventPool] = MaxNumEventsPerPool;
   } else {
     std::lock_guard<std::mutex> NumEventsAvailableInEventPoolGuard(
         NumEventsAvailableInEventPoolMutex);
@@ -373,12 +374,24 @@ _pi_context::getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &ZePool,
   return PI_SUCCESS;
 }
 
-pi_result
-_pi_context::decrementAliveEventsInPool(ze_event_pool_handle_t ZePool) {
-  std::lock_guard<std::mutex> Lock(NumEventsLiveInEventPoolMutex);
-  --NumEventsLiveInEventPool[ZePool];
-  if (NumEventsLiveInEventPool[ZePool] == 0) {
+pi_result _pi_context::decrementUnreleasedEventsInPool(pi_event Event) {
+  ze_event_pool_handle_t ZePool = Event->ZeEventPool;
+  std::lock_guard<std::mutex> Lock(NumEventsUnreleasedInEventPoolMutex);
+  --NumEventsUnreleasedInEventPool[ZePool];
+  if (NumEventsUnreleasedInEventPool[ZePool] == 0) {
     ZE_CALL(zeEventPoolDestroy, (ZePool));
+    // Nullify ZeEventPool pointer to indicate this pool is already destroyed
+    // because we will call ZeEventPoolDestroy() if ZeEventPool is not null
+    // in pi_context::finalize().
+    // Note that calling ZeEventPoolDestroy() for the already destroyed pool
+    // will cause a segmentation fault in L0.
+    // We need to check the equality below because it is possible that
+    // multiple pi_context::ZeEventPool can be created if all slots in the pool
+    // are already used up. So nullifying pi_context::ZeEventPool may point
+    // a  different EventPool than Event->ZeEventPool.
+    if (ZeEventPool == Event->ZeEventPool)
+      ZeEventPool = nullptr;
+    Event->ZeEventPool = nullptr;
   }
   return PI_SUCCESS;
 }
@@ -597,9 +610,9 @@ pi_result _pi_context::finalize() {
   // This function is called when pi_context is deallocated, piContextRelase.
   // There could be some memory that may have not been deallocated.
   // For example, zeEventPool could be still alive.
-  std::lock_guard<std::mutex> NumEventsLiveInEventPoolGuard(
-      NumEventsLiveInEventPoolMutex);
-  if (ZeEventPool && NumEventsLiveInEventPool[ZeEventPool])
+  std::lock_guard<std::mutex> NumEventsUnreleasedInEventPoolGuard(
+      NumEventsUnreleasedInEventPoolMutex);
+  if (ZeEventPool)
     ZE_CALL(zeEventPoolDestroy, (ZeEventPool));
 
   // Destroy the command list used for initializations
@@ -4419,7 +4432,7 @@ pi_result piEventRelease(pi_event Event) {
     ZE_CALL(zeEventDestroy, (Event->ZeEvent));
 
     auto Context = Event->Context;
-    if (auto Res = Context->decrementAliveEventsInPool(Event->ZeEventPool))
+    if (auto Res = Context->decrementUnreleasedEventsInPool(Event))
       return Res;
 
     // We intentionally incremented the reference counter when an event is

sycl/plugins/level_zero/pi_level_zero.hpp

@@ -328,7 +328,7 @@ struct _pi_context : _pi_object {
       : ZeContext{ZeContext},
         OwnZeContext{OwnZeContext}, Devices{Devs, Devs + NumDevices},
         ZeCommandListInit{nullptr}, ZeEventPool{nullptr},
-        NumEventsAvailableInEventPool{}, NumEventsLiveInEventPool{} {
+        NumEventsAvailableInEventPool{}, NumEventsUnreleasedInEventPool{} {
     // Create USM allocator context for each pair (device, context).
     for (uint32_t I = 0; I < NumDevices; I++) {
       pi_device Device = Devs[I];
@@ -429,8 +429,8 @@ struct _pi_context : _pi_object {
   pi_result getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &, size_t &);
 
   // If event is destroyed then decrement number of events living in the pool
-  // and destroy the pool if there are no alive events.
-  pi_result decrementAliveEventsInPool(ze_event_pool_handle_t pool);
+  // and destroy the pool if there are no unreleased events.
+  pi_result decrementUnreleasedEventsInPool(pi_event Event);
 
   // Store USM allocator context(internal allocator structures)
   // for USM shared and device allocations. There is 1 allocator context
@@ -459,21 +459,21 @@ struct _pi_context : _pi_object {
   // by storing number of empty slots available in the pool.
   std::unordered_map<ze_event_pool_handle_t, pi_uint32>
       NumEventsAvailableInEventPool;
-  // This map will be used to determine number of live events in the pool.
-  // We use separate maps for number of event slots available in the pool.
-  // number of events live in the pool live.
+  // This map will be used to determine number of unreleased events in the pool.
+  // We use separate maps for number of event slots available in the pool from
+  // the number of events unreleased in the pool.
   // This will help when we try to make the code thread-safe.
   std::unordered_map<ze_event_pool_handle_t, pi_uint32>
-      NumEventsLiveInEventPool;
+      NumEventsUnreleasedInEventPool;
 
   // TODO: we'd like to create a thread safe map class instead of mutex + map,
   // that must be carefully used together.
 
   // Mutex to control operations on NumEventsAvailableInEventPool map.
   std::mutex NumEventsAvailableInEventPoolMutex;
 
-  // Mutex to control operations on NumEventsLiveInEventPool.
-  std::mutex NumEventsLiveInEventPoolMutex;
+  // Mutex to control operations on NumEventsUnreleasedInEventPool.
+  std::mutex NumEventsUnreleasedInEventPoolMutex;
 };
 
 // If doing dynamic batching, start batch size at 4.