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.

Sign up for GitHub

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

Fix an error when compiling a watchOS target not supporting Thread-local storage. #5188

Merged
merged 5 commits into from
Feb 8, 2022
Merged

Fix an error when compiling a watchOS target not supporting Thread-local storage. #5188

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e651f7d
Fix an error when compiling a watchOS target not supporting Thread-lo…
dianaafanador3 Jan 24, 2022
759cdb8
Add condition only for watchOS simulator target
dianaafanador3 Jan 26, 2022
592951e
Merge branch 'master' into dp/watchos_target_not_thread_local
dianaafanador3 Feb 2, 2022
c904f46
Merge branch 'master' into dp/watchos_target_not_thread_local
dianaafanador3 Feb 4, 2022
cec69dd
Merge branch 'master' into dp/watchos_target_not_thread_local
dianaafanador3 Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* If a list of objects contains links to objects not included in the synchronized partition, the indices contained in CollectionChangeSet for that list may be wrong ([#5164](https://github.com/realm/realm-core/issues/5164), since v10.0.0)
* Sending a QUERY message may fail with `Assertion failed: !m_unbind_message_sent` ([#5149](https://github.com/realm/realm-core/pull/5149), since v11.8.0)
* Subscription names correctly distinguish an empty string from a nullptr ([#5160](https://github.com/realm/realm-core/pull/5160), since v11.8.0)
* Fix an error when compiling a watchOS target not supporting Thread-local storage ([#7623](https://github.com/realm/realm-swift/issues/7623))

### Breaking changes
* None.
Expand Down
2 changes: 1 addition & 1 deletion src/realm/impl/simulated_failure.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ bool SimulatedFailure::do_check_trigger(FailureType failure_type) noexcept
return false;
}

#if REALM_ARCHITECTURE_X86_32 && REALM_IOS
#if (REALM_ARCHITECTURE_X86_32 && REALM_IOS) || REALM_WATCHOS
Copy link
Contributor

Choose a reason for hiding this comment

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

This is related to #5102. I am going to make a guess and say that we should probably include REALM_TVOS as well. Alternatively, we already have a RLM_NO_THREAD_LOCAL which we could move to features.h so it could be used here as well.

Copy link
Contributor Author

@dianaafanador3 dianaafanador3 Jan 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The second option will help to avoid this to happen in other targets that we are not counting on, so will be a better solution to this issue. I can do it on this PR if we agree on this approach.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this approach but seems that it doesn't work for this particular scenario

Copy link
Member

Choose a reason for hiding this comment

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

RLM_NO_THREAD_LOCAL is equal to REALM_PLATFORM_APPLE, which doesn't seem like a correct definition.

Copy link
Member

Choose a reason for hiding this comment

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

Is thread-local storage actually not supported at all on watchOS? I would have expected it to be supported on 64-bit watchOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It Is not supported on the simulator, device architecture should be arm64_32 which I just read should support it
I could just check for simulator architecture
(REALM_ARCHITECTURE_X86_64 && REALM_WATCHOS)
or just for for architecture
REALM_ARCHITECTURE_X86_64
I'm not sure if the availability only depends of the architecture and/or platform.

bool (*s_mmap_predicate)(size_t);
#else
thread_local bool (*s_mmap_predicate)(size_t);
Expand Down