[android] Swap std::ostringstream usage for std::to_string

[sjchmiela]

Why

Eric reported native crashes when running older experiences on Expo client after RN 0.62 upgrade.

How

The first problem I had was reproducing it — after some testing @wkozyra95, @tsapeta and I realized it was only happening on Android 10.

The stacktrace didn't show too much information, but there was the most interesting part:

2020-05-28 11:19:42.552 25672-25672/? A/DEBUG:       #07 pc 000b6d6c  /data/app/host.exp.exponent-ikw7l6wjtBDnnLdjc-HnfQ==/lib/x86/libfbjni.so (__cxa_throw+108) (BuildId: 3546c1351f83366cb88b3802c4c63ee907e2db98)
2020-05-28 11:19:42.552 25672-25672/? A/DEBUG:       #08 pc 0007f919  /data/app/host.exp.exponent-ikw7l6wjtBDnnLdjc-HnfQ==/lib/x86/libc++_shared.so (std::__ndk1::locale::use_facet(std::__ndk1::locale::id&) const+217) (BuildId: 4824c9732515c02a2d10dbd0aff9cfee79138cbe)
2020-05-28 11:19:42.552 25672-25672/? A/DEBUG:       #09 pc 00027924  /data/app/host.exp.exponent-ikw7l6wjtBDnnLdjc-HnfQ==/lib/x86/libjscexecutor_abi37_0_0.so (std::__ndk1::basic_ostream<char, std::__ndk1::char_traits<char>>::operator<<(void const*)+148) (BuildId: 6f33b70e6648e4230566cae72b44d88a308d2dd8)
2020-05-28 11:19:42.552 25672-25672/? A/DEBUG:       #10 pc 00024557  /data/app/host.exp.exponent-ikw7l6wjtBDnnLdjc-HnfQ==/lib/x86/libjscexecutor_abi37_0_0.so (facebook::jsc::JSCRuntime::description()+407) (BuildId: 6f33b70e6648e4230566cae72b44d88a308d2dd8)

It looked like it threw a native exception when… building a description for JSCRuntime. The implementation of this method is:

std::string JSCRuntime::description() {
  if (desc_.empty()) {
    desc_ = std::string("<JSCRuntime@") + to_string(this) + ">";
  }
  return desc_;
}

where to_string is defined as

// std::string utility
namespace {
std::string to_string(void* value) {
  std::ostringstream ss;
  ss << value;
  return ss.str();
}
} // namespace

The use_facet methods from c++_shared aren't very helpful either:

const locale::facet*
locale::__imp::use_facet(long id) const
{
#ifndef _LIBCPP_NO_EXCEPTIONS
    if (!has_facet(id))
        throw bad_cast();
#endif  // _LIBCPP_NO_EXCEPTIONS
    return facets_[static_cast<size_t>(id)];
}

const locale::facet*
locale::use_facet(id& x) const
{
    return __locale_->use_facet(x.__get());
}

Then, fortunately, I realized I saw a PR removing std::stream usage recently — #8352.

I decided to follow its example and see whether changing:

 std::string JSCRuntime::description() {
   if (desc_.empty()) {
-    desc_ = std::string("<JSCRuntime@") + to_string(this) + ">";
+    desc_ = std::string("<JSCRuntime@") + std::to_string(reinterpret_cast<std::uintptr_t>(this)) + ">";
   }
   return desc_;
 }

could let the crash go, so I checked out the sdk-37 branch, applied the change, rebuilt the host.exp.reactandroid-abi37_0_0, copied maven folder, checked out master, pasted maven folder, rebuilt Expo client and lo and behold, SDK 37 didn't crash anymore! Opening SDK36 experience didn't crash anymore either!

Then I started testing more and I realized Expo client still crashed if the first experience opened after running the app was a not-patched SDK (eg. SDK36). So I went on and applied the fix for all other SDKs.

I also tried building projects with newer NDK (RN 0.62 uses r19c, this didn't help either).

If I understand the problem correctly, only one version of some symbol is used in Expo client—the one that is loaded first. For some reason the version loaded by unversioned Home doesn't count. Moreover, <<-ing to std::ostringstream may cause bugs in some circumstances.

I understand that this may fix the problem and nobody will experience any crash whatsoever anymore or this may fix only one probable cause of crashes and we only QA and users may figure out whether it's safe now.

Thanks to @wkozyra95 for assistance and answers during the whole day!

Test Plan

Running Expo client on Android 10 emulator does not cause a crash anymore no matter experience of which SDK is run first.

[esamelson]

Thanks for your amazing detective work on this @sjchmiela and @wkozyra95 . 🕵️ 🕵️

If I understand the problem correctly, only one version of some symbol is used in Expo client—the one that is loaded first. For some reason the version loaded by unversioned Home doesn't count.

This makes me think it's worth starting a serious conversation (perhaps after the SDK 38 release) about alternatives to having multiple SDK versions in the Expo client (at least the Android client). The setup we have now is hacky at best and it seems like it's just going to be an uphill battle to keep it working as RN continues to evolve. We shouldn't have to be going back and recompiling every old SDK version every time we add a new one.

But for now, let's land this so we can move ahead as planned, and hope that more similar issues do not crop up this time around 🙏