Skip to content

Data Races in IterableEmbeddedManager #902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
macbellingrath opened this issue Feb 24, 2025 · 1 comment · May be fixed by #912
Open

Data Races in IterableEmbeddedManager #902

macbellingrath opened this issue Feb 24, 2025 · 1 comment · May be fixed by #912

Comments

@macbellingrath
Copy link

Hello, there are several data races in the SDK as of version 6.5.11.

As an example, IterableEmbeddedManager stores some memory for placements and their associated messages. This memory is not protected and is accessed from multiple threads. This can lead to memory corruption and instability. Access to this memory should be synchronized if it accross threads.

class IterableEmbeddedManager: NSObject, IterableInternalEmbeddedManagerProtocol {
    ... 
    private var messages: [Int: [IterableEmbeddedMessage]] = [:]
   ...
    public func getMessages() -> [IterableEmbeddedMessage] { << not isolated
        return Array(messages.values.flatMap { $0 })
    }
...    
      private func retrieveEmbeddedMessages(completion: @escaping () -> Void) {
        apiClient.getEmbeddedMessages()
            .onCompletion(
                receiveValue: { embeddedMessagesPayload in
               ..
                    let processor = EmbeddedMessagingProcessor(currentMessages: self.messages, <<<< URLSession-Delegate Queue 
                                                               fetchedMessages: fetchedMessagesDict)
...
    private func setMessages(_ processor: EmbeddedMessagingProcessor) { << not isolated, called from several different threads
        messages = processor.processedMessagesList() 
        cleanUpTrackedMessageIds(messages)
    }
...
    
    public func reset() {
        let processor = EmbeddedMessagingProcessor(currentMessages: self.messages, fetchedMessages: [:])
        self.setMessages(processor)  << Message access on main thread 
@jena-chakour
Copy link
Contributor

Hey @macbellingrath thanks for surfacing this, our team will take this note into our next internal discussion on upcoming improvements. Let me know in our Support thread if anything misbehaves.

@Ayyanchira Ayyanchira linked a pull request Apr 25, 2025 that will close this issue
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 a pull request may close this issue.

2 participants