freshOS / Then

:clapper: Tame async code with battle-tested promises
MIT License
993 stars 77 forks source link

Thread explosion causing a deadlock? #68

Open roisg opened 4 years ago

roisg commented 4 years ago

Hi, we are using Then together with Vapor for a swift server side project. While doing some performance tests, we noticed that our current implementation is not able to support more than 64 concurrent requests without collapsing and being stuck in what it seems to be a deadlock.

For each request, we are always creating an async block and inside we'll be calling multiple async methods with await.

I've been able to reproduce the same issue using one of your tests. Our methods for each requests are similar to the testAsyncAwaitChainWorks one in AsyncAwaitTests class. If you modify that method to do the same code 100 times, it will look like this:

func testAsyncAwaitChainWorks() {
  for _ in 0..<100 {
    let exp = expectation(description: "")
    print("starting new task")
    async {
      print("starting async block")
      let userId = try await(fetchUserId())
      XCTAssertEqual(userId, 1234)
      let userName = try await(fetchUserNameFromId(userId))
      XCTAssertEqual(userName, "John Smith")
      let isFollowed = try await(fetchUserFollowStatusFromName(userName))
      XCTAssertFalse(isFollowed)
      exp.fulfill()
      print("finishing async block")
    }
  }
  waitForExpectations(timeout: 1000.0, handler: nil)
}

Executing this will print 100 times starting new task, but it will only print 64 times starting async block and finishing async block will not be printed at all, the execution will never finished.

Is there any way of avoiding this kind of issues and being able to use async/await approach while supporting a lot of concurrent requests?

antonm76 commented 3 years ago

Async is implemented with its own concurrent queue:

DispatchQueue(label: "then.async.queue", attributes: .concurrent).async

Looks like using one of the global queues could resolve it.