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

Fixed PHP memory leaks and arginfo errors #8614

Merged
merged 18 commits into from May 14, 2021
Merged

Conversation

@haberman
Copy link
Contributor

@haberman haberman commented May 14, 2021

This change makes us Valgrind-clean, and adds Kokoro CI tests to enforce that this remains true going forward.

Fixes: #8525

Specifically:

  • Fixed all known memory leaks and other memory errors, such that our test suite is Valgrind-clean, freeing all allocated memory (a few unresolved cases are disabled in the tests, mostly around exceptions).
  • Fixed several arginfo errors where we erroneously specified parameters to several functions.
  • Added a REFCOUNTING.md file to explain how refcounting works in the PHP C API.
  • Removed all uses of the confusing RETURN_ZVAL() macro.
  • Removed all explicit increment/decrement of refcounts in favor of having refcounts follow zvals.
  • We now explicitly add Descriptors to a list that is freed at the end of the request.
  • Removed a special case for descriptors of map entries by indexing the descriptor cache by upb_msgdef* instead of zend_class_entry*.
@fowles
fowles approved these changes May 14, 2021
@haberman haberman merged commit b0d90e3 into protocolbuffers:master May 14, 2021
54 checks passed
54 checks passed
@github-actions
Check for spelling errors
Details
@protobuf-kokoro
Bazel Kokoro build successful
Details
@protobuf-kokoro
Dist artifact installation Kokoro build successful
Details
@protobuf-kokoro
Linux 32-bit Kokoro build successful
Details
@protobuf-kokoro
Linux C# Kokoro build successful
Details
@protobuf-kokoro
Linux C++ Distcheck Kokoro build successful
Details
@protobuf-kokoro
Linux C++ TC Malloc Kokoro build successful
Details
@protobuf-kokoro
Linux Java JDK 7 Kokoro build successful
Details
@protobuf-kokoro
Linux Java Linkage Monitor Kokoro build successful
Details
@protobuf-kokoro
Linux Java Oracle 7 Kokoro build successful
Details
@protobuf-kokoro
Linux JavaScript Kokoro build successful
Details
@protobuf-kokoro
Linux PHP Kokoro build successful
Details
@protobuf-kokoro
Linux Python 2.7 Kokoro build successful
Details
@protobuf-kokoro
Linux Python 2.7 C++ Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.5 Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.5 C++ Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.6 Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.6 C++ Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.7 Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.7 C++ Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.8 Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.8 C++ Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.9 Kokoro build successful
Details
@protobuf-kokoro
Linux Python 3.9 C++ Kokoro build successful
Details
@protobuf-kokoro
Linux Python Release Kokoro build successful
Details
@protobuf-kokoro
Linux Ruby 2.4 Kokoro build successful
Details
@protobuf-kokoro
Linux Ruby 2.5 Kokoro build successful
Details
@protobuf-kokoro
Linux Ruby 2.6 Kokoro build successful
Details
@protobuf-kokoro
Linux Ruby 2.7 Kokoro build successful
Details
@protobuf-kokoro
Linux Ruby 3.0 Kokoro build successful
Details
@protobuf-kokoro
Linux Ruby Release Kokoro build successful
Details
@protobuf-kokoro
MacOS C++ Kokoro build successful
Details
@protobuf-kokoro
MacOS C++ Distcheck Kokoro build successful
Details
@protobuf-kokoro
MacOS JavaScript Kokoro build successful
Details
@protobuf-kokoro
MacOS Obj-C CocoaPods Integration Kokoro build successful
Details
@protobuf-kokoro
MacOS Obj-C OS X Kokoro build successful
Details
@protobuf-kokoro
MacOS Obj-C iOS Debug Kokoro build successful
Details
@protobuf-kokoro
MacOS Obj-C iOS Release Kokoro build successful
Details
@protobuf-kokoro
MacOS PHP7.0 Kokoro build successful
Details
@protobuf-kokoro
MacOS PHP7.3 Kokoro build successful
Details
@protobuf-kokoro
MacOS Python Kokoro build successful
Details
@protobuf-kokoro
MacOS Python C++ Kokoro build successful
Details
@protobuf-kokoro
MacOS Python Release Kokoro build successful
Details
@protobuf-kokoro
MacOS Ruby 2.4 Kokoro build successful
Details
@protobuf-kokoro
MacOS Ruby 2.5 Kokoro build successful
Details
@protobuf-kokoro
MacOS Ruby 2.6 Kokoro build successful
Details
@protobuf-kokoro
MacOS Ruby 2.7 Kokoro build successful
Details
@protobuf-kokoro
MacOS Ruby 3.0 Kokoro build successful
Details
@protobuf-kokoro
MacOS Ruby Release Kokoro build successful
Details
@mergeable
Mergeable Mergeable Run has been Completed!
Details
@protobuf-kokoro
Windows C# Kokoro build successful
Details
@protobuf-kokoro
Windows Csharp Release Kokoro build successful
Details
@protobuf-kokoro
Windows Python Release Kokoro build successful
Details
@google-cla
cla/google All necessary CLAs are signed
haberman added a commit to haberman/protobuf that referenced this pull request May 19, 2021
* Fixed a bunch of incorrect arginfo and a few incorrect error messages.

* Passes mem check test with no leaks!

* WIP.

* Fix build warning that was causing Bazel build to fail.

* Added compatibility code for PHP <8.0.

* Added test_valgrind target and made tests Valgrind-clean.

* Updated Valgrind test to fail if memory leaks are detected.

* Removed intermediate shell script so commands are easier to cut, paste, and modify.

* Passing all Valgrind tests!

* Hoist addref into ObjCache_Get().

* Removed special case of map descriptors by keying object map on upb_msgdef.

* Removed all remaining RETURN_ZVAL() macros.

* Removed all explicit reference add/del operations.

* Added REFCOUNTING.md to Makefile.am.
haberman added a commit that referenced this pull request May 19, 2021
* Some more updates to PHP testing infrastructure (#8576)

* WIP.

* Added build config for all of the tests.

* Use ../src/protoc if it is available, for cases where Bazel isn't available.

* Added test_php.sh.

* Fix for the broken macOS tests.

* Move all jobs to use php80 instead of lots of separate jobs.

* Only pass -t flag if we are running in a terminal.

* Updated php_all job to use new Docker stuff.

* Fixed PHP memory leaks and arginfo errors (#8614)

* Fixed a bunch of incorrect arginfo and a few incorrect error messages.

* Passes mem check test with no leaks!

* WIP.

* Fix build warning that was causing Bazel build to fail.

* Added compatibility code for PHP <8.0.

* Added test_valgrind target and made tests Valgrind-clean.

* Updated Valgrind test to fail if memory leaks are detected.

* Removed intermediate shell script so commands are easier to cut, paste, and modify.

* Passing all Valgrind tests!

* Hoist addref into ObjCache_Get().

* Removed special case of map descriptors by keying object map on upb_msgdef.

* Removed all remaining RETURN_ZVAL() macros.

* Removed all explicit reference add/del operations.

* Added REFCOUNTING.md to Makefile.am.

* Updated upb version and fixed PHP to not get unset message field. (#8621)

* Updated upb version and fixed PHP to not get unset message field.

* Updated changelog.

* Fixed preproc test to handle old versions of Clang withot __has_attribute().

* A second try at fixing __has_attribute().

* Copy __has_attribute() fix to cc file also.

* Updated failure list for PHP for fixed test.

* Updated version of upb for Ruby (#8624)

* Updated upb.

* Preserve legacy behavior for unset messages.

* Updated failure list.

* Updated CHANGES.txt.

* Added erroneously-deleted test file.

* Fixed condition on compatibility code.

* Re-introduced deleted file again, and fixed Rakefile to not delete it.

* Fix generation of test protos.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants