Fix memory safety issue

[tosh7]

経緯

Xcode15に対応した際に、CI上でテスト実行時にクラッシュが観測されることがあった。解析の結果MockingJayでクラッシュが起きている可能性が高く、MockingJayのレポジトリを見たところ修正のPRが作成されていた。

なぜForkして対応するのか

近年MockingJayのメンテナンスは止まっており(最終更新は3年前)、この修正がマージされる見込みがなかったのでこの修正をForkしたものでマージしてZOZOTOWNへと入れようと思っています。

方針について

最終的には現状を踏まえて、MockingJayからの依存を切るのが良いとは思いますが、このXcode15対応のタイミングではForkしてマージで対応としたいです。

[laprasdrum]

@tosh7 こちらの判断に至ったBitriseのbuild linkをいただけますか？

Xcode15に対応した際に、CI上でテスト実行時にクラッシュが観測されることがあった。
解析の結果MockingJayでクラッシュが起きている可能性が高く

[tosh7]

ちょっとBitriseで該当のログは見つからなかったのですが、
ローカルで発生していた時はこんな感じのログになっていました。
https://st-zozo.slack.com/archives/CDVDE8560/p1702640984283179

[tosh7]

Bitrise上で直近テスト実行時にクラッシュしているのだとここら辺ですね。
https://app.bitrise.io/build/3270bc54-c413-4c86-949b-80b6c23976b3?tab=log

↑ これに付随している問題かはログの詳しさ的に追えていませんんが。

[tosh7]

MockingJayがクラッシュする時のコンソールログ

Swift/ContiguousArrayBuffer.swift:600: Fatal error: Index out of range

[tosh7]

自分が作成して、approveは出せないのでコメントで。
おそらくクラッシュ時のログを追っていると、stubs.reversed()した後ものに対して回している最中にindex out of rangeが走っている。
ReversedCollectionは新しいインスタンスを作成するわけではないので、参照している最中にstubsに対して別の書き込みが発生したとかが可能性としてはある気がします。
Xcode15から発生するようになった理由はテストの並列実行の仕組みの変化か何かとかはあるかもしれないと思いました。
修正方法としては、より安全に使用できるようにDispatchQueuenにつめて、意図しないアクセスを防いでいるのでこの修正方法は良さそうな気がしました。

[tosh7]

上記の過程に基づいて、このような修正を入れるだけでテストがこけないようになりました。
reversed()したものをmapすることで、新しくArrayのインスタンスが作られるのでこんな感じでやっても動作としては担保することができる。

--- a/Pods/Mockingjay/Sources/Mockingjay/MockingjayProtocol.swift
+++ b/Pods/Mockingjay/Sources/Mockingjay/MockingjayProtocol.swift
@@ -67,7 +67,7 @@ public class MockingjayProtocol: URLProtocol {
   /// This method searches backwards though the registered requests
   /// to find the last registered stub that handles the request.
   class func stubForRequest(_ request:URLRequest) -> Stub? {
-    for stub in stubs.reversed() {
+      for stub in stubs.reversed().map({ $0 }) {
       if stub.matcher(request) {
         return stub
       }

[laprasdrum]

修正内容はこれでよさそうですが本家のindentは2なので合わせておきましょう

[laprasdrum]

Suggested change

	class MockingjayStorage {
	static let shared = MockingjayStorage()
	var stubs: [Stub] {
	get { accessQueue.sync { _stubs } }
	set { accessQueue.sync { _stubs = newValue } }
	}
	var registered: Bool {
	get { accessQueue.sync { _registered } }
	set { accessQueue.sync { _registered = newValue } }
	}
	private var _stubs = [Stub]()
	private var _registered: Bool = false
	private var accessQueue: DispatchQueue { DispatchQueue(label: "MockingjayStorageQueue", qos: .default) }
	}
	class MockingjayStorage {
	static let shared = MockingjayStorage()
	var stubs: [Stub] {
	get { accessQueue.sync { _stubs } }
	set { accessQueue.sync { _stubs = newValue } }
	}
	var registered: Bool {
	get { accessQueue.sync { _registered } }
	set { accessQueue.sync { _registered = newValue } }
	}
	private var _stubs = [Stub]()
	private var _registered: Bool = false
	private var accessQueue: DispatchQueue { DispatchQueue(label: "MockingjayStorageQueue", qos: .default) }
	}

[laprasdrum]

Suggested change

	private class var storage: MockingjayStorage { .shared }
	private class var stubs: [Stub] {
	get { storage.stubs }
	set { storage.stubs = newValue }
	}
	private class var registered: Bool {
	get { storage.registered }
	set { storage.registered = newValue }
	}
	private class var storage: MockingjayStorage { .shared }
	private class var stubs: [Stub] {
	get { storage.stubs }
	set { storage.stubs = newValue }
	}
	private class var registered: Bool {
	get { storage.registered }
	set { storage.registered = newValue }
	}

[tosh7]

これは、他人のBranchなのでCommitを積めなさそう。

[laprasdrum]

であれば仕方ないですがこのままmergeしましょうかね