From 38545553ec19f2707a972f448bf8a064827d746f Mon Sep 17 00:00:00 2001
From: Kevin Heifner <heifnerk@objectcomputing.com>
Date: Wed, 4 Oct 2023 14:23:21 -0500
Subject: [PATCH] GH-1716 pop before execute as the executed lambda can switch
 to read-only mode and spawn threads that access the queue.

---
 .../include/eosio/chain/exec_pri_queue.hpp    | 24 ++++++++++---------
 .../tests/custom_appbase_tests.cpp            |  4 ++--
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/libraries/custom_appbase/include/eosio/chain/exec_pri_queue.hpp b/libraries/custom_appbase/include/eosio/chain/exec_pri_queue.hpp
index 06c3208d7b..1f06f3db9e 100644
--- a/libraries/custom_appbase/include/eosio/chain/exec_pri_queue.hpp
+++ b/libraries/custom_appbase/include/eosio/chain/exec_pri_queue.hpp
@@ -53,17 +53,6 @@ class exec_pri_queue : public boost::asio::execution_context
       handlers_ = prio_queue();
    }
 
-   // only call when no lock required
-   bool execute_highest()
-   {
-      if( !handlers_.empty() ) {
-         handlers_.top()->execute();
-         handlers_.pop();
-      }
-
-      return !handlers_.empty();
-   }
-
 private:
    // has to be defined before use, auto return type
    auto pop() {
@@ -74,6 +63,19 @@ class exec_pri_queue : public boost::asio::execution_context
 
 public:
 
+   // only call when no lock required
+   bool execute_highest()
+   {
+      if( !handlers_.empty() ) {
+         auto t = pop();
+         bool empty = handlers_.empty();
+         t->execute();
+         return !empty;
+      }
+
+      return false;
+   }
+
    bool execute_highest_locked(bool should_block) {
       std::unique_lock g(mtx_);
       if (should_block) {
diff --git a/libraries/custom_appbase/tests/custom_appbase_tests.cpp b/libraries/custom_appbase/tests/custom_appbase_tests.cpp
index 75ccfeacac..945c77f8a6 100644
--- a/libraries/custom_appbase/tests/custom_appbase_tests.cpp
+++ b/libraries/custom_appbase/tests/custom_appbase_tests.cpp
@@ -40,7 +40,7 @@ BOOST_AUTO_TEST_CASE( default_exec_window ) {
    app->executor().post( priority::lowest, exec_queue::read_only, [&]() {
       // read_only_queue should only contain the current lambda function,
       // and read_write_queue should have executed all its functions
-      BOOST_REQUIRE_EQUAL( app->executor().read_only_queue().size(), 1); // pop()s after execute
+      BOOST_REQUIRE_EQUAL( app->executor().read_only_queue().size(), 0); // pop()s before execute
       BOOST_REQUIRE_EQUAL( app->executor().read_write_queue().size(), 0 );
       app->quit();
       } );
@@ -175,7 +175,7 @@ BOOST_AUTO_TEST_CASE( execute_from_both_queues ) {
    // stop application. Use lowest at the end to make sure this executes the last
    app->executor().post( priority::lowest, exec_queue::read_only, [&]() {
       // read_queue should have current function and write_queue's functions are all executed 
-      BOOST_REQUIRE_EQUAL( app->executor().read_only_queue().size(), 1); // pop()s after execute
+      BOOST_REQUIRE_EQUAL( app->executor().read_only_queue().size(), 0); // pop()s before execute
       BOOST_REQUIRE_EQUAL( app->executor().read_write_queue().size(), 0 );
       app->quit();
       } );