zopefoundation / transaction

Generic transaction implementation for Python.
Other
70 stars 31 forks source link

Still possible to join aborted and doomed transactions #85

Open jamadden opened 4 years ago

jamadden commented 4 years ago

Somewhat related to #64. Transaction.join functions the same whether a transaction is aborted or doomed or fully active. Should it allow that? What's the point of adding a resource manager to a transaction that can only be, or already has been, aborted?

On first glance this seems somewhat counterintuitive. I would have expected join to raise an exception like DoomedTransaction and AbortedTransaction instead of just allowing it. Maybe resource managers have already acquired resources before they make the call to join so it's important to be able to abort them later? But you can't currently abort transactions more than once, so that seems like an opportunity for bad behivour and leaks. join could automatically abort resource managers to cover that case.

def join(self, resource):
    if self.isAborted() or self.isDoomed():
        resource.abort()
        raise AbortedTransaction if self.isAborted() else DoomedTransaction
    # As before
    ...

The doom() documentation says the transaction is otherwise like normal, so for backwards compatibility this might be best limited to aborted transactions.