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

Patches sometimes do not apply #235

Closed
eseidel opened this issue Apr 6, 2023 · 9 comments · Fixed by shorebirdtech/updater#5
Closed

Patches sometimes do not apply #235

eseidel opened this issue Apr 6, 2023 · 9 comments · Fixed by shorebirdtech/updater#5
Assignees

Comments

@eseidel
Copy link
Contributor

eseidel commented Apr 6, 2023

I don't think we have an issue tracking this yet, so creating one.

This report came in from a trusted tester. It appears that sometimes the libapp.so path given to the engine does not exist on disk. It's probably a synthetic path that Android apis know how to magically extract from the APK and once they've done that later launches have the libapp.so on disk.

This only seems to happen from playstore installs, not from local installs.

@felangel has reproduced it with TimeShift install from the play store.

@eseidel eseidel converted this from a draft issue Apr 6, 2023
@eseidel eseidel self-assigned this Apr 6, 2023
@eseidel eseidel moved this from Todo to In Progress in v3 (multi-platform support + OAuth) Apr 6, 2023
@eseidel
Copy link
Contributor Author

eseidel commented Apr 6, 2023

An initial hypothesis was that it was related to minSdkVersion, but I believe @felangel tried and decided it repro'd with the default v16 from Flutter too?

@felangel
Copy link
Contributor

felangel commented Apr 6, 2023

Notes:

  • I was able to reproduce this using the latest version of Time Shift on the play store (which is still using minSdkVersion 16)
  • I was only able to reproduce using the downloaded version from the Play Store (local release builds worked fine).
  • After using adb pull I observed that the libapp.so is not present (only a bunch of top-level .apk files)
    • The /data/app/~~S1wBu43RLyNHHWENtw6Q_g==/dev.shorebird.u_shorebird_clock-yptLn3OK-L3gmvD5Hs6V3Q==/lib/arm64/ directory existed but was empty.

Logs

04-03 09:58:52.310 26721 26721 I flutter : updater::network: Patch check response: PatchCheckResponse { patch_available: true, patch: Some(Patch { number: 1, hash: "b21f7e43bb5bae406ef5efd740e1504beb8f456c7844f557bde31cd5c6ab5c64", download_url: "https://storage.googleapis.com/shorebird_patch_artifacts/51751336-6a7c-4972-b4ec-8fc1591fb2b3/android/aarch64/102/dlc.vmcode" }) }
04-03 09:58:52.311 26721 26750 D flutter : reqwest::connect: starting new connection: https://storage.googleapis.com/
04-03 09:58:52.382 26721 26750 D flutter : rustls::client::hs: No cached session for DnsName(DnsName(DnsName("storage.googleapis.com")))
04-03 09:58:52.382 26721 26750 D flutter : rustls::client::hs: Not resuming any session
04-03 09:58:52.416 26721 26750 D flutter : rustls::client::hs: Using ciphersuite TLS13_AES_256_GCM_SHA384
04-03 09:58:52.416 26721 26750 D flutter : rustls::client::tls13: Not resuming
04-03 09:58:52.416 26721 26750 D flutter : rustls::client::tls13: TLS1.3 encrypted extensions: [Protocols([6832])]
04-03 09:58:52.416 26721 26750 D flutter : rustls::client::hs: ALPN protocol is Some(b"h2")
04-03 09:58:52.483 26721 26750 D flutter : rustls::client::tls13: Ticket saved
04-03 09:58:52.483 26721 26750 D flutter : rustls::client::tls13: Ticket saved
04-03 09:58:52.631 26721 26721 I flutter : updater::updater: Patch is compressed, inflating...
04-03 09:58:52.631 26721 26721 I flutter : updater::updater: Reading base file: "/data/app/~~S1wBu43RLyNHHWENtw6Q_g==/dev.shorebird.u_shorebird_clock-yptLn3OK-L3gmvD5Hs6V3Q==/lib/arm64/libapp.so"
04-03 09:58:52.631 26721 26721 E flutter : updater::updater: Problem updating: Failed to open base file: "/data/app/~~S1wBu43RLyNHHWENtw6Q_g==/dev.shorebird.u_shorebird_clock-yptLn3OK-L3gmvD5Hs6V3Q==/lib/arm64/libapp.so"

@eseidel
Copy link
Contributor Author

eseidel commented Apr 6, 2023

Log from my run just now:

04-06 09:21:30.373 21068 21068 D flutter : updater::logging: Logging initialized
04-06 09:21:30.374 21068 21068 I flutter : updater::config: Updater configured with: ResolvedConfig { is_initialized: true, cache_dir: "/data/user/0/dev.shorebird.u_shorebird_clock/code_cache/shorebird_updater", download_dir: "/data/user/0/dev.shorebird.u_shorebird_clock/code_cache/shorebird_updater/downloads", channel: "stable", app_id: "51751336-6a7c-4972-b4ec-8fc1591fb2b3", release_version: "1.0.1", original_libapp_path: "/data/app/~~7dTPxkfP71eT1geBIVCgjg==/dev.shorebird.u_shorebird_clock-J_SIL5hquR3_vEP0zLXhEA==/lib/arm64/libapp.so", vm_path: "libflutter.so", base_url: "https://api.shorebird.dev" }
04-06 09:21:30.374 21068 21068 I flutter : [INFO:flutter_main.cc(123)] Starting Shorebird update
04-06 09:21:30.374 21068 21068 W flutter : updater::cache: Failed to load updater state: No such file or directory (os error 2)
04-06 09:21:30.375 21068 21068 I flutter : updater::network: Sending patch check request: PatchCheckRequest { app_id: "51751336-6a7c-4972-b4ec-8fc1591fb2b3", channel: "stable", release_version: "1.0.1", patch_number: None, platform: "android", arch: "aarch64" }
04-06 09:21:30.375 21068 21090 D flutter : reqwest::connect: starting new connection: https://api.shorebird.dev/
04-06 09:21:30.594 21068 21090 D flutter : rustls::client::hs: No cached session for DnsName(DnsName(DnsName("api.shorebird.dev")))
04-06 09:21:30.594 21068 21090 D flutter : rustls::client::hs: Not resuming any session
04-06 09:21:30.624 21068 21090 D flutter : rustls::client::hs: Using ciphersuite TLS13_AES_256_GCM_SHA384
04-06 09:21:30.625 21068 21090 D flutter : rustls::client::tls13: Not resuming
04-06 09:21:30.625 21068 21090 D flutter : rustls::client::tls13: TLS1.3 encrypted extensions: [Protocols([6832])]
04-06 09:21:30.625 21068 21090 D flutter : rustls::client::hs: ALPN protocol is Some(b"h2")
04-06 09:21:30.747 21068 21068 I flutter : updater::network: Patch check response: PatchCheckResponse { patch_available: true, patch: Some(Patch { number: 1, hash: "b21f7e43bb5bae406ef5efd740e1504beb8f456c7844f557bde31cd5c6ab5c64", download_url: "https://storage.googleapis.com/shorebird_patch_artifacts/51751336-6a7c-4972-b4ec-8fc1591fb2b3/android/aarch64/102/dlc.vmcode" }) }
04-06 09:21:30.748 21068 21095 D flutter : reqwest::connect: starting new connection: https://storage.googleapis.com/
04-06 09:21:30.798 21068 21095 D flutter : rustls::client::hs: No cached session for DnsName(DnsName(DnsName("storage.googleapis.com")))
04-06 09:21:30.798 21068 21095 D flutter : rustls::client::hs: Not resuming any session
04-06 09:21:30.825 21068 21095 D flutter : rustls::client::hs: Using ciphersuite TLS13_AES_256_GCM_SHA384
04-06 09:21:30.825 21068 21095 D flutter : rustls::client::tls13: Not resuming
04-06 09:21:30.825 21068 21095 D flutter : rustls::client::tls13: TLS1.3 encrypted extensions: [Protocols([6832])]
04-06 09:21:30.825 21068 21095 D flutter : rustls::client::hs: ALPN protocol is Some(b"h2")
04-06 09:21:30.854 21068 21095 D flutter : rustls::client::tls13: Ticket saved
04-06 09:21:30.854 21068 21095 D flutter : rustls::client::tls13: Ticket saved
04-06 09:21:31.191 21068 21068 I flutter : updater::updater: Patch is compressed, inflating...
04-06 09:21:31.191 21068 21068 I flutter : updater::updater: Reading base file: "/data/app/~~7dTPxkfP71eT1geBIVCgjg==/dev.shorebird.u_shorebird_clock-J_SIL5hquR3_vEP0zLXhEA==/lib/arm64/libapp.so"
04-06 09:21:31.191 21068 21068 E flutter : updater::updater: Problem updating: Failed to open base file: "/data/app/~~7dTPxkfP71eT1geBIVCgjg==/dev.shorebird.u_shorebird_clock-J_SIL5hquR3_vEP0zLXhEA==/lib/arm64/libapp.so"
04-06 09:21:31.191 21068 21068 E flutter : updater::updater: disabled backtrace
04-06 09:21:31.191 21068 21068 W flutter : updater::cache: Failed to load updater state: No such file or directory (os error 2)
04-06 09:21:31.191 21068 21068 I flutter : [INFO:flutter_main.cc(143)] Shorebird updater: no active patch.

@eseidel
Copy link
Contributor Author

eseidel commented Apr 6, 2023

I'm. not sure which version of the updater library the current Time Shift is using, but if it's current to head, then the fact that we don't see a message from "get_base_path" "File does not exist" would suggest the file does exist? Although I would expect to see a "file does not exist" from the first entry "libapp.so", suggesting this may not be fully up to date.

@eseidel
Copy link
Contributor Author

eseidel commented Apr 6, 2023

Felix and I discussed and decided we need to get this current release out (with all the updates to the updater logging) before we can debug this. So this will have to be punted to early next week for fixing. :(

@eseidel eseidel moved this from In Progress to Punt in v3 (multi-platform support + OAuth) Apr 6, 2023
@eseidel eseidel moved this from Punt to In Progress in v3 (multi-platform support + OAuth) Apr 7, 2023
@eseidel
Copy link
Contributor Author

eseidel commented Apr 7, 2023

We have a theory! We think this is just a string handling bug from C++.

the .sor.so is likely that it's re-using the same buffer as was used for libflutter.so with libapp.so but libapp.so is a shorter string and when overlayed is libapp.sor.so

We'll check our C++ string handling tomorrow, but I think we'll find the bug there.

eseidel added a commit to shorebirdtech/engine that referenced this issue Apr 7, 2023
Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.
eseidel added a commit to shorebirdtech/engine that referenced this issue Apr 7, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
@eseidel eseidel moved this from In Progress to Done in v3 (multi-platform support + OAuth) Apr 7, 2023
@eseidel
Copy link
Contributor Author

eseidel commented Apr 7, 2023

I believe this is fixed.

@eseidel eseidel closed this as completed Apr 7, 2023
@felangel felangel reopened this Apr 10, 2023
@felangel felangel self-assigned this Apr 10, 2023
@felangel felangel moved this from Done to In Progress in v3 (multi-platform support + OAuth) Apr 10, 2023
@felangel
Copy link
Contributor

felangel commented Apr 10, 2023

Reopening this since it appears we only addressed part of the issue. There still remains a scenario in which the libapp.so is not found. I'm investigating further now and will report back with more notes/observations.

Notes

  • It appears after installing the app from Google Play, the first couple installs indicate the libapp.so is missing however after a few restarts, eventually the libapp.so is found

  • Traced through dart_snapshot.cc and found that fml::NativeLibrary calls dlopen in the constructor

NativeLibrary::NativeLibrary(const char* path) {
  ::dlerror();
  handle_ = ::dlopen(path, RTLD_NOW);
  if (handle_ == nullptr) {
    FML_DLOG(ERROR) << "Could not open library '" << path << "' due to error '"
                    << ::dlerror() << "'.";
  }
}

@eseidel
Copy link
Contributor Author

eseidel commented Apr 11, 2023

I have a fix written: shorebirdtech/updater#5
but I can't test it locally since our dev flow broke after moving from --local-engine-src to artifact_proxy for our production builds it turns out they don't produce the same artifacts at least from an Arm Mac: flutter/flutter#124620.

I will figure out a workaround so I can test it.

@eseidel eseidel moved this to In Progress in 0.0.8: adding more TTs Apr 12, 2023
eseidel added a commit to shorebirdtech/updater that referenced this issue Apr 12, 2023
… it on disk (#5)

Fixes shorebirdtech/shorebird#235 🤞

This moves us away from depending on libapp.so having been extracted from the APK and instead we always extract it ourselves.
@github-project-automation github-project-automation bot moved this from In Progress to Done in 0.0.8: adding more TTs Apr 12, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Apr 12, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 6, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 6, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 6, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 6, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Jun 8, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Jun 8, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 8, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 8, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 16, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jun 16, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Jun 22, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Jun 22, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jul 13, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Jul 13, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Aug 18, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Aug 18, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Aug 24, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Aug 24, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Sep 8, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Sep 8, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Sep 19, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Sep 19, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Sep 21, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Sep 21, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Oct 2, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Oct 2, 2023
eseidel added a commit to shorebirdtech/engine that referenced this issue Oct 19, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
eseidel added a commit to shorebirdtech/engine that referenced this issue Oct 19, 2023
felangel pushed a commit to shorebirdtech/engine that referenced this issue Oct 26, 2023
* Fix unterminated string causing flaky behavior.

Fixes shorebirdtech/shorebird#235
The root cause was we were not terminating the strings passed
for original_libapp_paths.  We used a Vector<char> but didn't
append a '\0' to the end before calling data(), so when the c/rust
side walked to find the end of the char* it sometimes found extra
characters at the end of the string unexpectedly.

This fixes to use a shorter (but probably equally dangerous?)
method of using c_str() which we had tried before but previously
had used c_str() on a temporary.  This attempt carefully uses
a reference to the string so the c_str()'s lifetime is tied to
the string in application_library_paths rather than a temporary
copy.

This is still slightly dangerous in that c_str() lives as long
as a std::string does, so long as it's not modified.

* Fix comments
felangel pushed a commit to shorebirdtech/engine that referenced this issue Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
2 participants