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

Log dlopen errors in opt builds #41477

Merged
merged 4 commits into from
Apr 26, 2023
Merged

Conversation

jiahaog
Copy link
Member

@jiahaog jiahaog commented Apr 25, 2023

As the Engine uses dlopen to find the libapp.so, flutter/flutter#59834 can happen which will prevent dlopen from finding the binary. In theory this should be mitigated by #9762, but an internal customer observed errors that could be related.

The additional logging here can help to rule out that hypothesis. For Googlers, see b/276657840 for more details.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

As the Engine uses `dlopen` to find the `libapp.so`, flutter/flutter#59834 can happen which will prevent `dlopen` from finding the binary. In theory this should be mitigated by flutter#9762, but an internal customer observed errors where `dlopen` is not found. 

The additional logging here can help to rule out that hypothesis. For Googlers, see b/276657840 for more details.
@jiahaog jiahaog marked this pull request as ready for review April 25, 2023 10:25
@@ -13,8 +13,8 @@ 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() << "'.";
FML_LOG(ERROR) << "Could not open library '" << path << "' due to error '"
Copy link
Member

Choose a reason for hiding this comment

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

Since this will add a little misleading log spam on systems where falling back to the fully qualified path is expected, let's file an issue and add a TODO() here to shift this back to a DLOG when the investigation is complete.

@jiahaog jiahaog added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 26, 2023
@auto-submit auto-submit bot merged commit 321f801 into flutter:main Apr 26, 2023
@jiahaog jiahaog deleted the jiahaog-patch-3 branch April 26, 2023 08:56
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 26, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 26, 2023
…125549)

flutter/engine@87f5f4e...321f801

2023-04-26 jiahaog@users.noreply.github.com Log dlopen errors in opt builds (flutter/engine#41477)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jsimmons@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
jiahaog added a commit to jiahaog/flutter-engine that referenced this pull request Jul 14, 2023
This reverts commit 321f801.

This didn't seem to help with debugging b/276657840. Fixes flutter/flutter#125523.
auto-submit bot pushed a commit that referenced this pull request Jul 18, 2023
This reverts commit 321f801.

This didn't seem to help with debugging b/276657840. Fixes flutter/flutter#125523.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
This reverts commit 321f801.

This didn't seem to help with debugging b/276657840. Fixes flutter/flutter#125523.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants