Strange update / compatibility issue

Hi,

I did some more analysis and there is definitely something wrong with how your plugin handles the stock changes.

I now tested stock updates via:

• Woo product editor
• BEAR bulk editor plugin
• REST API
• direct PHP API calls when calling a page
• direct PHP API calls done asynchronously via cron

All of them work perfectly.
Only when stock updates come from Zettle, everything seems to be screwed for plugins listening to stock updates.

This might not be obvious if you test Zettle alone but as soon as there is another plugin which handles stock updates triggered by Zettle, things might become difficult.

I today tested several hours on a blank test environment just with Woo, Zettle, Woo BackInStockNotifier. The issue is 100% reproducible.

As you can imagine, it is quite difficult to understand exactly what is happening when looking into code which I did not write myself.

But I’d like to share what I found out, because this is quite interesting and points to the way how (or better when) your plugin handles stock updates.

The Woo Back-In-Stock (BIS) notifier plugin does sync its internal activities using WordPress shutdown hook.

When using one of the five successful stock update approaches above, I can see that

a) BIS hooks into the stock changes, updates it’s singleton queue and
b) upon shutdown-hook processes the queue for its solely proprietary activities.

On the other hand, when updating stock via Zettle, the sequence is:

a) the processing of the queue (triggered via shutdown hook) gets fired first(!!!) and then
b) the update hooks are being processed, filling the queue(!)

As your developers might now, the shutdown hook is being executed basically before PHP shuts down execution. After that happens, every subsequent activity is starting a new execution (thus creating new singletons).

In the above case, this means, that instead of the sync being executed as last activity, it gets executed _before_ the stock updates get added to the queue but then, PHP shuts down execution and the queue gets dismissed.

After having looked into your code (which I have to admit, I did not understand completely), I suspect this is happening:

You also hook into shutdown to make stock updates (at least in some cases). E.g. your QueueProcessor is hooked into the shutdown-hook.
This is the only way to explain what I am seeing right now.

And if you do that, this is a huge issue, because this way you create race conditions and issues with other plugins.

When you fire stock updates upon PHP shutdown, you hinder other plugins listening to these updates to properly process these.

If you fire them upon shutdown, you must expect that every piece of code listing to the events you initiated is doomed: you’re making critical updates after other plugins might already have been shut down by PHP (or better: their shutdown action already had been completed).

Could you kindly discuss this with your devs?
If I am right (and it really looks like it is happening just as I described), then you might basically break every other plugin which somehow is acting on stock updates.

Thanks for your support

Markus

• This reply was modified 2 years, 6 months ago by markisu72.
• This reply was modified 2 years, 6 months ago by markisu72.
• This reply was modified 2 years, 6 months ago by markisu72.

SUMMARY / CONCLUSION / SOLUTION

Summary

• The Zettle plugin uses the shutdown hook to execute stock updates
• During shutdown, the stock updates fire woocommerce_product_set_stock_status, woocommerce_variation_set_stock_status, woocommerce_product_set_stock, woocommerce_variation_set_stock
• Since those events get fired during shutdown, plugins listening to these events likely try to process these _AFTER_ they already have been shut down themselves

Conclusion

The Zettle plugin uses the shutdown hook not only to clean up some isolated stuff but to update data / fire stock update events in an already undefined system state. This must lead to incompatibilities and eventually even inconsistencies with other plugins listening to stock changes.

This is an architectural issue.

Solution

Updating stock during shutdown is a really dangerous practice, because you do that, after other plugins already have been shut down. It might work for you alone but it will 100% create issues with other stock-related plugins using a shutdown action.

Thus, the clean solution would require that you don’t update stock upon shutdown at all, but before.

A quick-fix (which still is not very clean but should work 99,999%) would be the following:

In WebhookListenerEndpoint.php, add PHP_INT_MIN as parameter when registering the shutdown action:

public function registerShutdownHook(Payload $payload): void
{
    /**
     * We need to signal 200 or Pusher will start to think the webhook is failing.
     * Hook into shutdown to postpone the actual processing until after the response is sent.
     */
    add_action(
        'shutdown',
        function () use ($payload): void {
            if (self::$shutDownCalled) {
                return;
            }

            $this->handle($payload);

            self::$shutDownCalled = true;
        },
        PHP_INT_MIN
    );
}

What this will do is keep your stock updates being executed during shutdown but it will make sure, that your shutdown action is executed prior to every other shutdown action.

By being the first plugin to shut down, you will trigger the stock update hooks _before_ all other plugins will shut down, thus allowing them to process them properly.

I tested this locally and can confirm, that this works.

So, this is just a 12-character-zero-risk-update of your code which will completely eliminate all compatibility / consistency issues created by updating the stock during shutdown.

Could you please include this into a quick update?
This is a really serious behavior and the effect is difficult to spot.
Thus, not heaving heard about this before does not mean, it does not happen.

Thanks
Markus