Added thread safety to session wrapper checks

[michael-wb]

What, How & Why?

There was a data race during client shutdown using force_close() that was occasionally reported by the TSAN evergreen tests. These changes add mutex thread safety around the parameters that can be modified by multiple threads.

Fixes #6844

☑️ ToDos

• 📝 Changelog update
• [ ] 🚦 Tests (or not relevant)
• [ ] C-API, if public C++ API changed.

[coveralls-official]

Pull Request Test Coverage Report for Build michael.wilkersonbarker_911

• 210 of 220 (95.45%) changed or added relevant lines in 2 files are covered.
• 143 unchanged lines in 15 files lost coverage.
• Overall coverage decreased (-0.02%) to 91.661%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/realm/sync/client.cpp	204	214	95.33%

Files with Coverage Reduction	New Missed Lines	%
src/realm/sync/client.cpp	1	91.57%
src/realm/sync/network/http.hpp	2	80.87%
src/realm/sync/network/websocket.cpp	2	75.15%
src/realm/uuid.cpp	2	97.06%
src/realm/sync/network/network.cpp	3	89.66%
test/fuzz_group.cpp	3	52.86%
src/realm/sync/transform.cpp	4	61.75%
src/realm/unicode.cpp	4	90.15%
src/realm/util/assert.hpp	4	87.1%
test/util/compare_groups.cpp	5	54.95%

Totals
Change from base Build 1805:	-0.02%
Covered Lines:	230722
Relevant Lines:	251711

---

💛 - Coveralls

[danieltabacaru]

New table is synced after migration is still failing. You can try the fix proposed in one of my PRs.

[danieltabacaru]

There was a data race during client shutdown using force_close() that was occasionally reported by the TSAN evergreen tests.

How was the race manifesting?

[kiburtse]

This is essentially very suspicious code when you guard just a few variables with very limited scope. Looks like this class serves multiple purposes... I'd need to look at it more carefully. It's not obvious it this fix just silence particular tsan warning or solves the issue with this class design and concurrent access.

[kiburtse]

From what i can tell, this fixes the linked issue with data race on force_close. I think though we should make this fix cleaner. @michael-wb do you have any plans for this change in near term?

[kiburtse]

Is this still true?

[kiburtse]

Maybe we can move these fields to some other class, and make it fully threadsafe? This is a bit unsettling that only part of this class is supposed to be accessed from different threads. It's error prone.