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

Default to WSDL_CACHE_DISK caching strategy #200

Closed
wants to merge 1 commit into from

Conversation

mlamm
Copy link

@mlamm mlamm commented Oct 30, 2018

The soap caching strategy WSDL_CACHE_MEMORY (WSDL_CACHE_BOTH) caused random segmentation faults while using this library, after debugging the issue we found that WSDL_CACHE_DISK is much more reliable and makes the crashes go away.

So Disk-Caching should be the default in here.

The soap caching strategy WSDL_CACHE_MEMORY (WSDL_CACHE_BOTH) caused random segmentation faults while using this library, after debugging the issue we found that WSDL_CACHE_DISK is much more reliable and makes the crashes go away.

So Disk-Caching should be the default in here.
@veewee
Copy link
Contributor

veewee commented Oct 30, 2018

Hi @mlamm,

Thanks for the pr!

Can you tell me more about the segfault?
I had similar issues which could be solved by increasing the soap.wsdl_cache_limit ini setting: http://php.net/manual/en/soap.configuration.php#ini.soap.wsdl-cache-limit

More info about the bug: https://bugs.php.net/bug.php?id=50722

As you state: caching to disk is better but I would prefer no caching by default. This way you can manually enable it or create your own cache file and store it in a faster caching backend like redis.
Iirc: the cache file is just a copy of the wsdl with all it's external xsd files downloaded into this file. But I have to reinvestigate that statement that be sure.

@mlamm
Copy link
Author

mlamm commented Nov 1, 2018

Hey veewee, thanks for taking a look at this.

Unfortunately we weren't able to track down the exact line and file of the php-code, i talked to the guys in #php on freenode and they suggested staying away from memory caching.

I spinned up some debug container to debug the crash, however, I always did run into a failing assertion on this line https://github.com/php/php-src/blame/php-7.2.11/ext/soap/php_sdl.c#L3024, that code is only used for memory caching, so I just turned it off completely. The failing assert is not necessarily the crash location, I never figured out where it's actually crashing since it also happened randomly.

I just wanted to have my founding flowing back to the soap lib here, to avoid other people running into the very same issue. However, changing default values can be tricky, since some users might rely on them.
Another thing that makes it confusing, the lib overwrites those flags always, so settings in php.ini or ini_set() are silently ignored.

@veewee
Copy link
Contributor

veewee commented Dec 6, 2018

I've set the default to disk caching in the new version (#197).
Closing this one for now.

@veewee veewee closed this Dec 6, 2018
@mlamm
Copy link
Author

mlamm commented Dec 12, 2018

Probably one of the underlying issues got fixed recently: https://bugs.php.net/bug.php?id=76348

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.

None yet

2 participants