Skip to content

Conversation

@ZStriker19
Copy link
Collaborator

Backport of #5747 to 1.10

When cleaning up the modules as part of the support for gevent, we also unload google.protobuf. However, starting with version 4.21, the new default upb backend does not tolerate being unloaded, which resulted in segfaults in applications with a runtime dependency on protobuf.

Checklist

  • Change(s) are motivated and described in the PR description.
  • Testing strategy is described if automated tests are not included in the PR.
  • Risk is outlined (performance impact, potential for breakage, maintainability, etc).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Library release note guidelines are followed.
  • Documentation is included (in-code, generated user docs, public corp docs).
  • PR description includes explicit acknowledgement/acceptance of the performance implications of this PR as reported in the benchmarks PR comment.

Reviewer Checklist

  • Title is accurate.
  • No unnecessary changes are introduced.
  • Description motivates each change.
  • Avoids breaking API changes unless absolutely necessary.
  • Testing strategy adequately addresses listed risk(s).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Release note makes sense to a user of the library.
  • Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.

When cleaning up the modules as part of the support for gevent, we also
unload google.protobuf. However, starting with version 4.21, the new
default upb backend does not tolerate being unloaded, which resulted in
segfaults in applications with a runtime dependency on protobuf.

(cherry picked from commit e98cb91)
@ZStriker19 ZStriker19 requested review from a team as code owners May 5, 2023 16:24
@ZStriker19 ZStriker19 requested review from Yun-Kim and emmettbutler May 5, 2023 16:24
@ZStriker19 ZStriker19 added the Profiling Continous Profling label May 5, 2023
@ZStriker19 ZStriker19 enabled auto-merge (squash) May 5, 2023 16:24
@pr-commenter
Copy link

pr-commenter bot commented May 5, 2023

Benchmarks

Comparing candidate commit f9ac3d1 in PR branch backport-5747-to-1.10 with baseline commit e07505f in branch 1.10.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 56 cases.

@ZStriker19 ZStriker19 merged commit 83e05d0 into 1.10 May 5, 2023
@ZStriker19 ZStriker19 deleted the backport-5747-to-1.10 branch May 5, 2023 16:49
@github-actions github-actions bot added this to the v1.10.4 milestone May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Profiling Continous Profling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants