Skip to content

[MOB-22537] - added processing of listeners asynchronously inside serial queue#126

Merged
akhiljain1907 merged 14 commits intoadobe:dev-v5.3.1from
akhiljain1907:fix/raceCondition
Apr 3, 2025
Merged

[MOB-22537] - added processing of listeners asynchronously inside serial queue#126
akhiljain1907 merged 14 commits intoadobe:dev-v5.3.1from
akhiljain1907:fix/raceCondition

Conversation

@akhiljain1907
Copy link
Copy Markdown
Contributor

@akhiljain1907 akhiljain1907 commented Jan 16, 2025

Description

This PR solves race condition issue observed in Optimize SDK.
Jira Link - MOB-22537

When a personalization request is made to edge, our propositionsInProgress dictionary is returned from parent's repsonseEvent before it gets updated in processEdgeResponse. We are now processing our listeners in a serial queue to solve this.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@akhiljain1907 akhiljain1907 marked this pull request as draft January 16, 2025 12:11
@akhiljain1907 akhiljain1907 changed the base branch from dev-v5.2.1 to dev-v5.3.0 March 10, 2025 14:43
@akhiljain1907 akhiljain1907 marked this pull request as ready for review March 10, 2025 15:30
@akhiljain1907 akhiljain1907 changed the title added processing of listeners asynchronously inside serial queue [MOB-22537] - added processing of listeners asynchronously inside serial queue Mar 21, 2025
"Cannot process the update propositions request event, Configuration shared state is not available.")
return
}
queue.async {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we capture [weak self] here as well to prevent a strong reference to self on line 203?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

- AEPCore (< 6.0.0, >= 5.0.0)
- AEPServices (< 6.0.0, >= 5.0.0)
- AEPCore (5.3.1):
- AEPCore (5.4.0):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's be sure these updated dependency versions are represented in the podspec and Package.swift files

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

XCTAssertEqual(1, updateEventIdsInProgress.count)
XCTAssertEqual([DecisionScope(name: "eyJhY3Rpdml0eUlkIjoieGNvcmU6b2ZmZXItYWN0aXZpdHk6MTExMTExMTExMTExMTExMSIsInBsYWNlbWVudElkIjoieGNvcmU6b2ZmZXItcGxhY2VtZW50OjExMTExMTExMTExMTExMTEifQ==")], updateEventIdsInProgress.values.first)
// using DispatchQueue to change the run loop as the events are now being processed inside a serial queue in optimize extension
DispatchQueue.main.async { [weak self] in
Copy link
Copy Markdown
Member

@sbenedicadb sbenedicadb Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better to run these lines on a global (bg) thread than the main thread

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using background thread, but on running the tests there were inconsistency as a few of them used to fail because of the same reason we used a DispatchQueue to change the run loop (Assertions are executed before the events get processed).

Copy link
Copy Markdown
Member

@sbenedicadb sbenedicadb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple small comments that need to be addressed

@akhiljain1907 akhiljain1907 changed the base branch from dev-v5.3.0 to dev-v5.3.1 March 28, 2025 06:48
@akhiljain1907
Copy link
Copy Markdown
Contributor Author

Hi @sbenedicadb thanks a lot for reviewing. I have addressed your comments and made relevant changes in the PR. Would request you to kindly check and review again.

@akhiljain1907 akhiljain1907 merged commit a2597ef into adobe:dev-v5.3.1 Apr 3, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants