Consider the following code:

public class OnlineShoppingService {

   @Transactional
   public void cancelOrder(String id) {
       if (shipmentRepository.findShipmentForOrder(id) != null) {
           throw new ConflictException("Shipped orders can not be cancelled!");
       }
       orderRepo.findById(id).setCancelled(true);
   }

   @Transactional
   public void ship(String orderId) {
       var order = orderRepo.findById(orderId);
       if (order.isCancelled()) {
           throw new ConflictException("Cancelled orders can not be shipped!");
       }
       shipmentRepo.add(new Shipment(order));
   }

(Order and Shipment are versioned JPA entities)

At first glance, this code seems to ensure that Shipments only exist for Orders that are not cancelled. Fresh out of university, I'd have been convinced it does ("transactions are atomic, which means indivisible. We can reason about them as if they had executed in sequence").

However, in the default isolation level of most databases, read committed, an Order can become cancelled during the execution of ship(). If that happens after the status is checked, but before we commit our transaction, a Shipment is created for a cancelled Order.

How do you prevent such bugs? How do you go about writing correct code and reasoning about its correctness? And how do you explain all that to junior software developers you are mentoring?

Do you

• hit them with the formal definition of isolation levels, explaining about non-repeatable reads, phantom reads, and ask them to verify that every method they write correctly handles all these phenomena? (seems very time consuming and error prone?)
• sidestep the problem by cranking up the isolation level to Serializable ? (I've never seen anyone do that?)
• impose an architecture / coding convention that prevents such errors?
• something else?

PS: I realize this is a rather broad topic; I'm happy to receive partial answers or links to outside references or even books.

posted over 3 years ago

CC BY-SA 4.0

3y ago

meriton‭

2375 reputation 7 56 220 35