WooCommerce Can’t Count Either

WooCommerce is up for the thread-safety test in this post and probably a couple of other to follow.

What changes with every sale most of the time? Product stock. In WooCommerce it’s decreased when an order is paid. The code responsible for this is currently located in WC_Product_Data_Store_CPT::update_product_stock. The docblock acknowledges that this method contains critical sections of code and that locking is used (spoiler: it’s not).

Uses queries rather than update_post_meta so we can do this in one query (to avoid stock issues). Uses locking to update the quantity. If the lock is not acquired, change is lost.

But looking at the actual body of the method you’re met (in version 4.0.1) with this comment:

// @todo: potential race condition. Read current stock level and lock the row. If the lock can’t be acquired, don’t wait.

Yikes! The lock was never implemented. It’s a tiny bit better than using a get_post_meta and update_post_meta chain, for one reason: doing direct database queries is faster; the race condition window is tinier. But essentially it’s the same: there are windows between fetching the value and updating it. So it’s still very vulnerable. Let’s prove this.

First, I created a product with 2000 stock quantity. Then I wrote a jMeter request scenario that orders this product as a new customer and pays for it via a mock payment gateway. Instead of writing a full scenario the same can be done with the following code as well:

// Create a random customer
$customer = new WC_Customer();
$customer->set_email( md5( random_bytes( 32 ) ) . '@sandbox.threadsafe.org' );
$customer_id = $customer->save();

$order = new WC_Order();
$order->set_customer_id( $customer_id );

$item = new WC_Order_Item_Product();
$item->set_quantity( 1 );
$item->set_product_id( 18 ); // A product with 2000 stock and no backorders

$order->add_item( $item );

$order->save();

$order->payment_complete(); // Paid and ready to go

After a pool of 20 concurrent shop sessions buys the product 100 times in succession my product is left with the following stock:

67 products left! This happened because while updating the stock concurrent threads overwrote the stock value with stale data. There are 2000 paid orders for the product, but there’s still 67 units of the product left.

What would this mean for your business? In this specific scenario you’ll have to speak to up to 67 customers. Tell them that you don’t have any stock left, oops. Or backorder and explain that they need to wait. If WooCommerce is integrated with your accounting software, or with your warehouse or dropshipper, then the problem further extends to third-parties.

Enough talk, though. There’s an evident problem.

What can be done?

I’ve opened a pull request in the WooCommerce GitHub repository. The fix is fairly straightforward. It only works for increment and decrement operations. I use direct SQL math operations instead of separate queries.

For the set operation this needs another strategy. Maybe use a FROM DUAL “upsertish” database statement, which is atomic. Very similar to how WP_Lock‘s wpdb backend functions. Luckily set operations are used only on the Product management screen.

The patch still doesn’t address a higher-level issue. The checkout mechanism has a race condition while placing the order, by the time it checks whether there’s enough stock a concurrent request would have already placed a valid order. This will result in negative stock quantities by a couple of stock units with my patch. But at least the stock value will match the number of orders to the dot.

Disclaimer

This is not a security threat.

This work was sponsored by threadsafe.org.