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

Conversation

haberman
Copy link
Member

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*.

@haberman haberman merged commit b0d90e3 into protocolbuffers:master May 14, 2021
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
Development

Successfully merging this pull request may close these issues.

PHP memory leak
3 participants