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 variable scoping issue which can lead to out of memory error. #9

Merged
merged 1 commit into from
Jun 13, 2014

Conversation

shaunhardy
Copy link
Contributor

The $properties variable I'm assuming was expected to go out of scope, however it does not. Each iteration through the SOAP types appends more elements to the $properties array without clearing the data from the previous iteration, leading to massive memory consumption and Out of Memory errors in some cases. I added an explicit reassignment to the $properties variable.

In my specific case, this consumed 171MB, and has been reduced to 2MB.

ddeboer added a commit that referenced this pull request Jun 13, 2014
Fixed variable scoping issue which can lead to out of memory error.
@ddeboer ddeboer merged commit 9f014c8 into phpforce:master Jun 13, 2014
@ddeboer
Copy link
Member

ddeboer commented Jun 13, 2014

I guess the problem was that the $properties variable wasn’t be defined at all. Good catch, thanks!

@shaunhardy
Copy link
Contributor Author

Yup, I'm seeing that as well. It looks like it's a bug in the PHP SOAP library. https://bugs.php.net/bug.php?id=45404

\SoapClient::__getTypes() is ignoring extensions. In the WSDL, Id is defined in the sObject, and extended in Account, etc.

@daum
Copy link

daum commented Aug 12, 2014

@ddeboer any chance you are going to merge this soon?

@ddeboer
Copy link
Member

ddeboer commented Aug 12, 2014

This PR is already merged. Can you open a new one?

@daum
Copy link

daum commented Aug 13, 2014

Ah disregard that comment - I misread @shaunhardy comment. Having the issue he is talking about in #11 . will follow up on that ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants