Skip to content
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

Tracker.subject is not thread safe #817

Closed
hakonk opened this issue Sep 4, 2023 · 3 comments
Closed

Tracker.subject is not thread safe #817

hakonk opened this issue Sep 4, 2023 · 3 comments
Labels
type:defect Bugs or weaknesses. The issue has to contain steps to reproduce.
Milestone

Comments

@hakonk
Copy link

hakonk commented Sep 4, 2023

Describe the bug
A SIGSEGV crash with signal 11 code 2 was discovered in production. Here are the three top calls in the stack trace:

Dictionary.count.getter
_dictionaryUpCast<T>
Tracker.addBasicProperties (Tracker.swift:522)

As can be seen in Tracker.swift:521-522, a dictionary belonging to subject is retrieved. Access to subject is not wrapped in any synchronisation mechanism, and I believe this is the root cause for the crash. SIGSEGV signal 11 code 2 also points to a memory related issue.

if let subjectDict = subject?.standardDict(userAnonymisation: userAnonymisation) {
     payload.addDictionaryToPayload(subjectDict) // <- crash happens here
}

To Reproduce
While I have not reproduced the crash in isolation, I believe one can achieve this by accessing the subject on one thread, while also tracking several other events on another thread.

Expected behavior
Not crash

Device information (please complete the following information):

  • Device: AppleTV5,3
  • OS: tvOS
  • Version 16.6
@hakonk hakonk added the type:defect Bugs or weaknesses. The issue has to contain steps to reproduce. label Sep 4, 2023
@matus-tomlein
Copy link
Contributor

Hi @hakonk, thank you for reporting this crash! We will look into getting this fixed.

@jeffremer
Copy link

jeffremer commented Sep 25, 2023

We ran into what I think is the same or a similar crash just as we were about to push the change out to production, but luckily our internal beta testers caught it. I made some recent changes to how we wrap and call Snowplow as well as introduced setting the current userID on the tracker's subject prior to sending each event - previously we were not manually setting any properties on the subject.

Unfortunately we're not able to reproduce the crash reliably either, but the various crash groupings either point to the same Tracker.swift lines that @hakonk has pointed out or callsites that reference Subject.swift. Aside from the same call site called out here, my theory about this being the same derives from the hints that all the crashing seems to occur off the main thread when modifying the subject which is characteristically different from how we've used the tracker in the past (where it just seems that incidentally most events were sent on the main thread).

Screenshot 2023-09-25 at 3 29 20 PM

Screenshot 2023-09-25 at 3 30 05 PM

Screenshot 2023-09-25 at 3 32 20 PM

(SnowplowTracker is statically linked as an SPM package into our own wrapper module called Analytics which is why the module column of the stack looks a little wrong).

@matus-tomlein
Copy link
Contributor

Fixed in version 6.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:defect Bugs or weaknesses. The issue has to contain steps to reproduce.
Projects
None yet
Development

No branches or pull requests

3 participants