[GitHub] [arrow] westonpace commented on pull request #13091: ARROW-16498: [C++] Fix potential deadlock in arrow::compute::TaskScheduler

[GitBox <gi...@apache.org>]

westonpace commented on PR #13091:
URL: https://github.com/apache/arrow/pull/13091#issuecomment-1120093156

   CC @michalursa 
   
   `TaskScheduler, StressTwo` should reproduce this but it can still be a bit of a pain on a fast system.  Added a print statement between:
   ```
     const auto& tasks = PickTasks(num_new_tasks);
   ```
   and
   ```
     if (static_cast<int>(tasks.size()) < num_new_tasks) {
   ```
   adds enough of a delay to trigger this fairly reliably (you will also have to comment out the fix).
   
   This fix should be a pretty light-touch but it does add one compare_and_swap in the `ScheduleMore` method.  Another potential fix would be to turn the following lock into a full on spinlock:
   ```
     for (;;) {
       int expected = num_tasks_to_schedule_.value.load();
       if (num_tasks_to_schedule_.value.compare_exchange_strong(expected, 0)) {
         num_new_tasks += expected;
         break;
       }
     }
   ```
   changing it to:
   ```
     for (;;) {
       int expected = num_tasks_to_schedule_.value.load();
       if (expected == 0) {
         continue;
       }
       if (num_tasks_to_schedule_.value.compare_exchange_strong(expected, 0)) {
         num_new_tasks += expected;
         break;
       }
     }
   ```
   
   Let me know if you want to try anything different.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org