Fix a PHP memory leak with SSL root certificates

[jackwakefield]

I've been running into an issue with requests to PHP-FPM increasing memory by ~100MB which doesn't get freed.

I ran strace on PHP-FPM and noticed many calls to map vendor/grpc/grpc/etc/roots.pem into memory, narrowed this down to grpc_ssl_roots_override_result and PHP_METHOD(ChannelCredentials, setDefaultRootsPem) mallocing default_pem_root_certs without freeing, so replaced this with grp_realloc and made sure to make a copy of the value in grpc_ssl_roots_override_result.

I based this on the C# extension

grpc/src/csharp/ext/grpc_csharp_ext.c

Line 901 in 8399f92

static grpc_ssl_roots_override_result override_ssl_roots_handler(

This appears to be related to #11632

[grpc-testing]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-testing]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-testing]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-testing]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-testing]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[stanley-cheung]

Ok to test

[RyanGordon]

Does this need a mutex around it? Not entirely sure if this is correct but potentially multiple PHP requests can be setting the default credentials at any point. Before it wouldn't be an issue since the memory would just leak and still be valid but now that it is being re'alloced it could cause problems?

[skormel]

@jackwakefield Your patch solve ours memory leaks problems.

I think this patch must be included on the main tree.

Thanks you very much.

[thelinuxfoundation]

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

• User @jackwakefield isn't covered by a CLA. They will need to complete the form at https://identity.linuxfoundation.org/projects/cncf

Regards,
The Linux Foundation CLA GitHub bot

[jackwakefield]

Signed.

[ZhouyihaiDing]

Thank you for helping us find it.
The code looks good to me. I am going to make a test before merging it.
May I ask how you observe the memory usage? Using some php tools or read from the process directly?

[jackwakefield]

I first picked it up in top after noticing performance degrading after each request, I then used strace as described here and noticed a large number of mmap calls mapping vendor/grpc/grpc/etc/roots.pem.

[ZhouyihaiDing]

Jenkins, test this please

[ZhouyihaiDing]

Sorry for the delay.
I run the tests in ubuntu and mac and it works well.
For the question about the lock, it's probably fine because setDefaultRootsPem is seldom called.
I think it's ready for merging after jenkins run the tests.
Thank you!

[ZhouyihaiDing]

LGTM