search

BottleRocketStudios / iOS-Hyperspace

An extremely lightweight wrapper around URLSession to make working with APIs a breeze.
Apache License 2.0
47 stars 17 forks source link

EXC_BAD_ACCESS in TransportService.swift:46 #144

Closed GrandLarseny closed 2 years ago

GrandLarseny commented 2 years ago

It appears that Hyperspace isn't thread safe. I've encountered this scenario a few times where executing multiple requests asynchronously will result in sporadic EXC_BAD_ACCESS errors at TransportService.swift:46. See the following screenshot:

LLDB access error

As you can see in the screenshot, there is another thread in the execute() function when the crash occurs. I believe that this error is occurring because of a data race inside the execute function when modifying the tasks variable.

My first suggestion is to change TransportService to be an actor instead of class. This should alleviate the problems with concurrent network executions.

Of course, this means that TransportService will be asynchronous, which is a pretty big overhaul. I do understand if this seems to big a break, even for a major release.

So, skipping back a beat, my second suggestion will be to do all tasks work on a dispatch queue, wrapping all references to tasks in an async block. Similar to the approach suggested here: https://www.swiftbysundell.com/articles/swift-actors

Open to suggestions!

wmcginty commented 2 years ago

I think for now the better solution would be to wrap access to tasks in a serial DispatchQueue to minimize the surface of changes.

I would imagine that fully adopting the new Swift concurrency model will bring about a larger set of changes that make sense for a 5.0 release.

GrandLarseny commented 2 years ago

Yeah, sounds good to me. I'll make the changes and submit a PR.

GrandLarseny commented 2 years ago

PR https://github.com/BottleRocketStudios/iOS-Hyperspace/pull/145 ready for review

wmcginty commented 2 years ago

Fixed in #145 .