Handling multiple asynchronous events - Wait for pending offers to process on new offer?

Question

I have a programming problem, that I don't know how to solve. And while I have provided a sample of my code, I am interested in a conceptual answer on how to resolve this problem.

On a tradeOffers event, I call a function to get a list of offers and process them one by one using async.forEachOfSeries from Async.js module. However, if a new offer comes in while pending offers are still processing then the same offer can be processed multiple times and also return an offer unavailable error because it was already accepted. This is a problem because the new offer is effectively not evaluated.

Is it possible to solve this with simple callback or true / false flag, or would I need to implement a queue for the offers? If I need to use a queue, then I would like to avoid using SQL to store the offers. Or do I need a more complex control mechanism to handle new tradeOffers events when processing is already in place?

I am currently using steam-tradeoffers and steam-user module.

To better explain my current situation, here is the code:

    steam.on('tradeOffers', function(count) {
        if (count > 1 && bot_working == true) {
            processoffers();
        }
        });

    function processoffers() {
        if (bot_working) {
            offers.getOffers({
                get_received_offers: 1,
                active_only: 1,
                get_sent_offers: 0,
                get_descriptions: 1,
                time_historical_cutoff: Math.round(Date.now() / 1000),
                language: "en_us"
            }, function(error, body) {
                if(error) return;
                if(body.response.trade_offers_received){
                async.forEachOfSeries(body.response.trade_offers_received, function(offer, key, cboffer) {
                        if (offer.trade_offer_state == 2){
                            //process offer
                        }
                        }, function (err) {
                if (err) console.log(err.message);
                  console.log('Completed all incoming offers.');
                }
            ); //function foreach
        } //if trade tradeoffers
    }); //if getoffers
};
};

Answer 1

If the array you're asynchronously processing can be modified by other code, problems like this are bound to happen at some point. The simplest way to prevent this kind of race condition is to make a defensive copy.

The "multiple acceptance" bug you describe makes it sound like this callback is mutating the actual objects in the array, rather than merely changing which objects the array contains, so you'll probably have to use a deep copy rather than a shallow copy for this to work.

Ideally, the defensive copy would be done in the implementation of getOffers() before calling the callback, so that call sites never have to worry about these problems. Personally, I always throw in a defensive copy whenever I implement a method like this, because it preemptively prevents so many subtle bugs in client code. Whether I use a deep or a shallow copy depends on how much control I'm supposed to have over the objects I'm passing to client code.

---

But sometimes, the array or the objects it contains might be too large for a deep defensive copy to be practical. In that case, you're going to need a more interesting solution. I don't know if this applies to your problem, but for the sake of completeness...

1. You could try to ensure that tradeOffers never runs while there is an unfinished getOffers() request. As you suggested, the most straightforward way of doing this is to maintain a queue of requests and process them one at a time. But even that approach will require the getOffers() callback to somehow tell you when it's done executing all of its async bits. Relying on client code to always return a promise or always call a special callback when it's really really done is...risky to say the least.
2. A quick hacky solution would be to stick to fully synchronous code inside the getOffers() callback. However, this relies on the assumption that getOffers() calls your callback synchronously rather than asynchronously, otherwise you still have one tick where a tradeOffers event can sneak in. Maybe you happen to know that getOffers() always calls its callbacks synchronously. But personally, even if I did know that, this sort of subtle timing assumption makes me uneasy because it comes a bit too close to "releasing Zalgo".
3. You could store the offers in an actual database, since most databases have already solved this problem.

In case it isn't obvious, I'd go with a database when the data is too big to copy. Relying on databases to do heavy lifting tends to prevent a lot of headaches.