[CakePHP3] トランザクションが予期せずロールバック／コミットされる深刻な不具合

## 概要

先日、 CakePHP3 においてトランザクションが予期せずロールバック／コミットされてしまうという深刻な不具合が報告されました。

問題の概要把握のために、まずは下記のコードをご覧ください。

```php
$this->loadModel('Bookmarks');
$this->Bookmarks->connection()->transactional(function(){
    $this->Bookmarks->findOrCreate(['user_id' => -1]);
    $this->Bookmarks->Users->findOrCreate(['id' => null]);
    return false;
});
```

このコードでは transactional() 中で二度の findOrCreate() を呼び出した後、最後に false を返すことで意図的にロールバックさせています。つまり、結果としては主キーの自動採番が進む程度で基本的には**何も起こらないことが期待されます**。

しかし、現実は異なります。 users テーブルに**1件のレコードが挿入されます**。

これは最初の findOrCreate() の呼び出しで**トランザクションが予期せずロールバック**しており、次の findOrCreate() の実行は**トランザクションが張られていない状態**、つまり**即時コミットされる**ために発生している事態です。

この例に登場するテーブルは、公式の[ブックマークチュートリアル](https://book.cakephp.org/3.0/ja/tutorials-and-examples/bookmarks/intro.html#id3)の定義を使用し、ただ bake したものですので、どなたでも簡単に再現が可能です。実際に試してみてください。

## 影響バージョン

3.0.0 から 3.4.2 までの全てのバージョン

## 発生条件

### 前提

Connection::useSavePoints(true) を呼び出してセーブポイントを有効にしていない（既定値：無効）
※なお、このメソッドは CakePHP 3.4 以降ではバージョンでは enableSavePoints(true) というメソッドになっています。

### 条件

上記の前提において、下記の条件を満たした場合に発生します。

1. トランザクション (T1) を開始する
2. 入れ子のトランザクション (T2) を開始する
3. 入れ子のトランザクション (T2) をロールバックする
4. トランザクション (T1) を抜ける前に更新クエリを実行する

※実際には 3. の時点でトランザクション (T1) は終了しているため 4. のクエリは即時コミットされてしまいます。

この条件を冒頭の実際の例に照らしてみましょう。

1. まず transactional() の呼び出しで条件 1. を満たします。
2. 次に最初の findOrCreate() の呼び出しにより条件 2. を満たします。これは findOrCreate() が既定で[内部的にトランザクションを開始する](https://github.com/cakephp/cakephp/blob/c428da92979bd7f0367a18e5b55146592a2c9c51/src/ORM/Table.php#L1505-L1507)ためです。
3. また、この呼び出しは同時に条件 3. も満たすことになります。引数として ['user\_id' => -1] という無効な外部キーを渡しており、これがアプリケーションルールの ExistsIn に抵触するためです。ここで入れ子のトランザクションによりロールバックが実行されることになります。
4. そして二番目の findOrCreate() の呼び出しにより、条件 4. を満たしたということになります。

## 発生理由

トランザクションの入れ子関係は、大別して以下の4種類があります。

```sql
-- COMMIT のみが行われる
BEGIN
    (BEGIN)
    (COMMIT)
COMMIT

-- ROLLBACK のみが行われる
BEGIN
    (BEGIN)
    (ROLLBACK)
ROLLBACK

-- 内側に COMMIT を含み、外側で ROLLBACK が行われる
BEGIN
    (BEGIN)
    (COMMIT)
ROLLBACK

-- 内側に ROLLBACK を含み、外側で COMMIT が行われる
BEGIN
    (BEGIN)
    (ROLLBACK)
COMMIT
```

この内、問題のある入れ子関係は最後の1つで、内側でロールバックが発生したトランザクションが最終的に外側でコミットされる種類ののものです。

セーブポイントが無効になっている場合、 CakePHP はこれをエミュレートしようとします。しかし、内側のトランザクションのロールバックをエミュレートするのは困難です。直前のクエリだけを取り消すような命令はないからです。

では、実際にフレームワークはどのように処理するのでしょうか。

まず、以前のバージョンである CakePHP2 に関して言えば、内側のロールバックは無視されます。

```sql
BEGIN
    (BEGIN)
        INSERT tags SET title = 'hogehoge';
    (ROLLBACK)

    INSERT tags SET title = 'fugafuga';
COMMIT
```

上の例では最初の INSERT が取り消されることはなく、結果的に2件のタグが挿入されることになります。つまり、内側でコミットを呼んだのと変わらない結果になっています。

CakePHP3 の実装はこれとは異なります。挿入されるレコードは1行だけです。これは一見、正しいふるまいのように思われますが、しかして、その実態は恐ろしいことになっています。

```sql
BEGIN
    (BEGIN)
        INSERT tags SET title = 'hogehoge';
ROLLBACK

INSERT tags SET title = 'fugafuga';
COMMIT
```

あろうことか ROLLBACK を実際に発行して**外側のトランザクションを閉じてしまう**のです。続く INSERT は COMMIT を待たずに即時コミットされます。いったい、誰がこの結果を予想したでしょうか。

したがって CakePHP3 においては、エミュレートするのが難しい4番目の入れ子関係のみならず、何ということのない、2番目のロールバックのみが行われる入れ子関係さえも期待通りには動作しません。

```sql
BEGIN
    (BEGIN)
        INSERT tags SET title = 'hogehoge';
    (ROLLBACK)

    INSERT tags SET title = 'fugafuga';
ROLLBACK
```

この例では COMMIT は一度も実行していませんが、結果的には1件のタグが挿入されることになります。

```sql
BEGIN
    (BEGIN)
        INSERT tags SET title = 'hogehoge';
ROLLBACK

INSERT tags SET title = 'fugafuga';
ROLLBACK
```

最初の ROLLBACK の時点でトランザクションは終了していますから、本来、すべてをなかったことにするはずだった二番目の ROLLBACK は、もはや後の祭り、直前の INSERT を取り消すことはできません。

[ロールバックの実装](https://github.com/cakephp/cakephp/blob/3.4.2/src/Database/Connection.php#L493-L494)をご覧いただければ、そのように処理されることがおわかりになるだろうと思います。

ちなみに、これはロールバック固有の仕様で、[コミットの実装](https://github.com/cakephp/cakephp/blob/3.4.2/src/Database/Connection.php#L465)はそうなってはいません。コミットとロールバックについて行われる条件がそれぞれ異なる、何とも非対称な実装になっています。

冒頭の例に戻ると、 findOrCreate() が内部的にトランザクションを開始することは述べた通りですが、普通は次のようなトランザクションを期待します。

```sql
-- transactional()
BEGIN

    -- findOrCreate()
    (BEGIN)
    (ROLLBACK)

    -- findOrCreate()
    (BEGIN)
    (COMMIT)

ROLLBACK
```

しかし、実際にデータベース上で起きていたことは次の事態だったのです。

```sql
-- transactional()
BEGIN

    -- findOrCreate()
    (BEGIN)
ROLLBACK

-- findOrCreate()
BEGIN
COMMIT

ROLLBACK
```

## 影響を受けるコード

findOrCreate() の他に、内部的に入れ子のトランザクションを作りうるメソッドには把握している限り以下のようなものがあります。

**Connection**

- transactional()

**Table**

- save() ※atomic オプションに false を渡さなかった場合
- delete() ※atomic オプションに false を渡さなかった場合
- findOrCreate() ※atomic オプションに false を渡さなかった場合
- saveMany()

**BelongsToMany**

- link()
- unlink()
- replaceLinks()

**HasMany**

- link()

**TreeBehavior**

- removeFromTree()
- moveUp()
- moveDown()
- recover()

こうしたメソッドをトランザクション中で使用している場合、予期せぬロールバックが発生する恐れがあり、以降は即時コミットの状態になります。なお、内部的にこれらを呼び出すメソッドについては調査していませんので、影響のあるメソッドはこれよりも遥かに多いでしょう。

ちなみに replaceLinks() というのは、 BelongsToMany のアソシエーションを save() する際、 saveStrategy が append でない場合（既定値は replace です）に呼ばれるメソッドです。すると、 BelongsToMany のアソシエーションを保存する場合、 save() と replaceLinks() で条件の 1. と 2. はすでに満たしており、もしも何らかの理由で保存に失敗すれば 3. を満たして、即時コミット状態に突入してしまうことになります。

実際、次の例ではトランザクション中に save() を一度呼び出しただけですが、 bookmarks テーブルに予期しないレコードが挿入されてしまいます。

```php
$this->loadModel('Users');

// タグの保存を失敗させるイベント
$rules = $this->Users->Bookmarks->Tags
    ->eventManager()->on('Model.beforeSave', function($event){
        return false;
    });

$this->Users->connection()->transactional(function(){
    $user = $this->Users->get(1);

    $this->Users->patchEntity($user, [
        'bookmarks' => [
            [ 'title' => 'hogehoge', 'tags' => [ ['title' => 'dummy'] ] ],
            [ 'title' => 'hogehoge' ],
        ],
    ], ['associated' => ['Bookmarks.Tags']]);

    $this->Users->save($user, ['atomic' => false, 'associated' => ['Bookmarks.Tags']]);

    return false;
});
```

## 回避策

残念ながら存在しません。当初はセーブポイントを有効にすることで回避できると考えていましたが、アプリケーションの作りによっては[大量のデッドロックが発生する可能性](https://github.com/cakephp/cakephp/issues/10348#issuecomment-284343381)があるようで、本稿ではお勧めはしません。

また atomic に false を渡して入れ子のトランザクションを作らないようにすることも、場合によっては逆効果で、直前の例では false を渡したためにレコードの挿入が行われてしまいます。これは先に列挙した通り replaceLinks() にはこの設定は無効であり、加えて atomic に false を渡すと ORM の挙動が変わり、 [更新に失敗しても復帰せず処理を続行してしまう](https://github.com/cakephp/cakephp/blob/3.4.2/src/ORM/Table.php#L1846)ためです。

save() 等を呼び出した場合に必ず返値を確認し、適切にエラーを処理することでこの問題を軽減することはできますが、直前の例のように save() から復帰する前に問題が発生する場合にはこれも不可能です。

## 修正

### 3.4 以降のバージョン

この問題を受けて、私の方で Connection クラスに[修正コミット](https://github.com/cakephp/cakephp/pull/10353)を行っています。3.4 をご利用の方は 3.4.3 以降へのアップグレードをお勧めします。

ちなみに修正内容としては、先に述べた4種類の入れ子の中で、問題のある4番目の入れ子が発生した場合にのみ例外を投げるという処理になっています。

```sql
BEGIN
    (BEGIN)
    (ROLLBACK)
COMMIT -- 例外が投げられます。
```

上の例では、外側のトランザクションが COMMIT で閉じられた場合に NestedTransactionRollbackException という例外が投げられます。最後が ROLLBACK で閉じられた場合にはこの例外は投げられません。

```sql
BEGIN
    (BEGIN)
    (ROLLBACK)
ROLLBACK -- 例外は投げずに false を返して復帰します。
```

ちなみに、投げられた例外のスタックトレースは、そのトランザクション中で発生した最初のロールバックを指していますので、ロールバックの発生原因の調査にご利用になれると思います。

なお、当初は rollback() を根本的に修正するつもりでいたのですが、実はこのふるまいは[早期ロールバックアルゴリズム](https://github.com/cakephp/cakephp/blob/3.4.2/tests/TestCase/Database/ConnectionTest.php#L499-L519)なる仕様であることがわかり、これに依存している既存のアプリケーションへの配慮のために断念しました。この修正で予期せぬロールバックか発生しなくなるのは transactional() のみです。

したがって、直接 rollback() を呼び出している箇所があれば、そのふるまいは修正後も変更されません。呼び出した瞬間に一撃でトランザクションが終了します。

もっとも rollback() の引数として新たに $toBeginning という一撃でトランザクションを終了させるかどうかを指定する boolean のオプションを追加していますので、 rollback() を直接使っていて、かつ入れ子のトランザクションを一段階だけ閉じたい場合には rollback(false) のように呼び出してください。

前述の通り、早期ロールバックアルゴリズムに依存しているアプリケーションを守るために 3.4 の時点ではこのオプションの既定値は true です。

ただし、既定値については次のマイナーリリースである 3.5 では変更される可能性がありますし、個人的には false に変更したいと思っています。万が一、一撃でトランザクションを閉じるために rollback() を使用されている方がいらっしゃいましたら、今後は明示的に true を渡すようにしてください。

### 3.4 以前のバージョン

この問題は脆弱性ではないため 3.4 以前のバージョンについては公式に修正は行われませんが、問題の深刻さに鑑みて非公式ながら私の方で[修正パッチ](https://github.com/chinpei215/cakephp-patch)を用意しています。 3.4 にアップグレードできない方はよろしければこちらをご利用ください。

## 謝辞

この不具合は2017年3月1日、 [CakePHP公式Slack日本語チャンネル](http://qiita.com/chinpei215/items/3c116171c5308365c314)内で @icchii さんによって報告された問題について、私の方で追加調査および修正を行ったものです。 @icchii さんには[セグメンテーション違反の問題](http://qiita.com/zaramme/items/df18270fffec548f4900)でも貴重なご助言をいただき、たいへん感謝しております。本稿の最後にこの場を借りて御礼を申し上げます。