Aws-sdk-php: Memory Leakage

[sm2017]

I have a long execute php script (Executes for months)

I find that we have a memory leakage in aws-sdk-php , When I use GetObjectAsync method of S3 client , and after minutes I run the garbage collector , these objects are in memory

GuzzleHttp\Psr7\Request
GuzzleHttp\Psr7\Stream
GuzzleHttp\Psr7\Uri
Aws\RetryMiddleware
Aws\S3\PutObjectUrlMiddleware
Aws\S3\PermanentRedirectMiddleware
Aws\Command
Aws\HandlerList

[kstich]

Hi @sm2017, could you please provide a little more information, or potentially a small reproduction, of your issue at hand? That will help us to investigate further what's causing your issue and either guide adjustments or work on updates. Thanks!

[sm2017]

Run the following code

<?php
require 'vendor/autoload.php';

$sdk = new Aws\Sdk([
    'version' => 'latest',
    'region'  => 'us-west-2'
]);

$s3 = $sdk->createS3();
$promise = $s3->getObjectAsync([
    'Bucket' => 'bucket',
    'Key' => 'memoryLeakage',
]);

$promise->wait();
echo "Before gc_collect_cycles()\n\n\n";
gc_collect_cycles();
echo "\n\n\nAfter gc_collect_cycles()";

Please add __destruct method to these classes

GuzzleHttp\Psr7\Request
GuzzleHttp\Psr7\Stream
GuzzleHttp\Psr7\Uri
Aws\RetryMiddleware
Aws\S3\PutObjectUrlMiddleware
Aws\S3\PermanentRedirectMiddleware
Aws\Command
Aws\HandlerList

public function  __destruct(){
    echo __METHOD__."\n";
}

With no memory leakage you must not see anything between Before gc_collect_cycles() and After gc_collect_cycles()

As my script is long execute , I see that these objects never free up memory even after days

[stof]

Cyclic objects graph are not collected as soon as the variable goes out of scope (as the refcount never reaches 0), but the next time PHP decides to run the GC to delete unused cyclic graphs. gc_collect_cycles is a way to force PHP to run it now instead of waiting for PHP to run it (which may be delayed until PHP considers that it needs to free some memory).

An actual memory leakage would not release memory during GC. It would just keep increasing.

If you have disabled the GC in your php.ini, cyclic object graphs would indeed create memory leaks, but then it is your fault (as you disabled the mechanism aimed at releasing memory).

[stof]

However, there is still a room for optimization in the SDK here to try to remove the cyclic object graph (so that refcounting can release the memory faster)

[kstich]

We're still investigating this issue and potential changes in relation to it. I'm attaching some PHP documentation links here in case they are of help.

Collecting Cycles
Performance Considerations

[imshashank]

@ssigwart @stof @sm2017 Thanks for reporting the issue. We will add this our backlog to reduce PHP SDK memory footprint.

In the meantime, please feel free to share any strategies, code or examples you have to solve this issue or to reduce memory footprint.

[ssigwart]

Using gc_collect_cycles() after every few calls when uploading large amounts of data to S3 works well to free the memory for me.

[sm2017]

The best way to free up memory is assigning null to variable when its garbage

For anonymous function remember this note

Run this code
```php
class A{
public $a;

function __destruct(){
    echo "This object(A) removed from memory<br/>\n";
}

}

class B{
public $B;

function __destruct(){
    echo "This object(B) removed from memory<br/>\n";
}

}

$a = new A;
$a->a = function()use($a){
//unset($a);
};

$b = new B;
$b->b = $a;

unset($a,$b);
echo "You think both \$a and \$b are freed from memory But \$a is in memory
\n";
````

So it is safe to use unset inside anonymous function

As you see object of class A is in memory and destructed in shutdown

[imshashank]

@sm2017 @ssigwart @stof We will be closing this issue as this has more to do with how PHP handles cyclic object graphs than with the SDK. We will also add a task to out backlog to look at how PHP uses memory and optimize it.

Please feel free to add any comments or reopen the issue if you see any memory leaks as a result of PHP SDK.

[Xylane]

Hello,

I'm sorry but I don't understand why this issue is closed.

Issue exists since more than 1 year, and it's not just a question of "how PHP handles cyclic object graphs" : the function uploadPart() is used in combination with createMultipartUpload(), which is made especially to upload large files. So it HAS to be "optimised" for this usage, and actually it isn't.

Moreover, the script given as example in your official documentation is generating fatal error (allowed memory size exhausted), as soon as the memory limit for this script is lower than the size of the file you wish to upload (even when GC is enabled).

In my humble opinion, this issue is caused by a bad usage of PHP features, and must be addressed : this is developer's job to know how PHP is handling cyclic object graphs and to adapt his script accordingly in order to avoid fatal errors.

Finally, if you considers that this issue cannot be fixed because of PHP some limits (let me have huge doubts, since for example high-level API doesn't have this issue for example), you shoud at least document this issue in your official website, list possible solutions and update your example accordingly (for example by using gc_collect_cycle()).

In any case, there is something to do. And closing the issue without doing nothing is not a valid "solution"...

Regards,

Lionel

[dinamic]

Guys, there's definitely a problem in the library. One of our cron job workers dies on regular basis with no stacktrace and no information to debug. However, the out of memory error happens in GuzzleHttp\Psr7\Stream::__toString(), which is called by the AWS PHP SDK.

We are using streams as in php streams. There seem to be some flaw that would not use the buffer you intended to use for big files, and would cause the stream to be casted to string, which does put the whole file in memory.

[thebeanieman]

Im a web developer with 16 years experience and today I wasted 5 hours dicking around trying to get Amazon S3 to upload a 78G file by following amazons documentation and code examples. It was only after reading this issue that I was able to upload the file by using using gc_collect_cycle() as suggested. I was working on a server that had cpanel and php installed, I could not tweak the config. I think it is VERY important that this issue is looked into again as not everybody can change the host configuration from the default of their host to one that suits Amazon. The memory was not being released in the loop and increased with each iteration until it met the server limits. I could set ini_set('memory_limit','78000M'); even if the server had that much ram and it would still FAIL. This really needs looking into rather than just closing it saying change your server config to suit the API.

[Xylane]

I totally agree with you thebeanieman, but it's soooo much easier to propose crappy code and to state that "issue is related to PHP" than to spend some time in order to check where to put some "unlink" or "unset" function in order to properly release the memory :-/
This issue has been opened almost 2 years ago, and nothing has been done. Example given by the official documentation to "upload large object" continue to return fatal error when uplooading large object, and "developers" of Amazon continue to consider that it's not an issue.

Congrats guys !

[thebeanieman]

I understand there code might be good practice, but what good is that if people that are not server admins can't use it? The entire point of an API is to HELP PEOPLE todo something. It's not like people deliberately turned settings off and came moaning.

[Xylane]

And anyway the garbage collector is just a help. Look at C or C++ languages : there is no garbage collector and you must release memory manually.
When you use properly a programming language and you have devent skills, you always release the memory manually. You don't only count on garbage collector that is more random and can create side effect.
Garbage collector is just a plan B to avoid crash in case of "poorily coded app" or if you forgot to release some memory.

In this case, it's clearly stated that the example given in the official documentation is made for uploading large files. How is it possible not to release memory after uploading each part ? What is the logic of splitting a large file in small parts if you need at least as much memory as the full file to upload ???

The only reason is that Amazon developers don't want to "waste" time fixing their own code ! I'm sorry but it's unacceptable.

[thebeanieman]

When I was 15 I started a JAVA course, I was good at programming but I gave up as I was confused about having to deal with memory allocation before I even used a variable!

I agree about all of your comments. Over the years I have learned not to presume anything as people have different environments. Im very good at saying I have 16 years experience and boosting, but you know what, I only know what I know, im not a Webmaster yet, and the way things are going with all these Fancy Frameworks, I never will be. I can do PHP thou! ha.

Im not here to rant, im here to offer my help and advice to help get this fixed so other people are not left pulling there hair out wondering why they follow the documentation and it does not work! I have had all sorts of curl issues today, tried loads. I agree the code should reset the variables / memory by itself. Maybe thats not in PSR standards? I just make good code that works lol

[Xylane]

PSR will never say not to release memory. Trust me :)

It's not a question of best practice, and even if it was official documentation should mention in its code how to use its own framework properly (which is not the case).

The issue is that Amazon's developer prefer to state that "PHP garbage collector is not doing its job" than to write clean code which will not depend on PHP garbage collector :(

[thebeanieman]

I worked on a website host with a Magento site, the garbage collector never ran, they resulted in using a cron to delete the millions of session files ! ha

[Xylane]

Haha yes, managing memory is not easy. That's why recent programming language include garbage collector to help developers.
But you shouldn't 100% rely on it. It's just giving some help but it cannot do everything instead of you. In the current situation it seems clear that garbage collector is not able to release memory automatically. Fine, and what? Does it prevent developers to release memory manually? No, it doesn't.

So please, Amazon, fix it by yourself !

[diehlaws]

Thank you for your contributions to the discussion on this issue @dinamic, @thebeanieman, and @Xylane. I'm re-opening the issue to bring it up during our sprint to the rest of the team for further investigation on this behavior.

[thebeanieman]

No problem. Please let me know if you need any tests done or any clarification.

[Xylane]

Thanks diehlaws, really.

As @thebeanieman, don't hesitate to inform us if I can help to test some patch !

[howardlopez]

We've added documentation to the S3 Multipart Uploader guide that gives an example of manually invoking the cycle collection between part uploads. Due to the performance impact of invoking the algorithm, we don't necessarily want the SDK to do this automatically for the user, especially when there's a hook that allows the user to specify it when needed.

We're still investigating memory management in the SDK, but if there are any other specific pain points caused by the memory footprint of the cyclic references (besides the multipart uploads), please let us know. In any case, we're appreciative of the feedback!

[howardlopez]

Since this is another potential pain point, we've added documentation to the CommandPool guide that gives a similar example of manual cycle collection between operations.

[tedchou12]

Hi. Sorry for interrupting here.
I am also facing memory leak issue here trying to use the Stream interface within aws sdk:

function stream($file_path='', $file_name='', $file_size=0, $file_type='')
    {
      header('Content-Type: '. $file_type);
      header('Accept-Ranges: bytes');

      $headers = $this->s3->headObject(array(
        'Bucket'               => $this->bucket,
        'Key'                  => $this->folder.'/'.$file_path,
      ));

      $size   = $file_size;     // File size
      $size   = $headers['ContentLength'];
      $length = $size;           // Content length
      $start  = 0;               // Start byte
      $end    = $size - 1;       // End byte
      // header('X-Custom:'.$_SERVER['HTTP_RANGE']);
      // header('Content-Disposition: inline; filename="' . rawurlencode($file_name) . '"');

      $this->s3->registerStreamWrapper();
      $file = "s3://{$this->bucket}/{$this->folder}/{$file_path}";
      $context = stream_context_create(array('s3' => array('seekable' => true)));

      if (isset($_SERVER['HTTP_RANGE'])) {
        $c_start = $start;
        $c_end   = $end;
        list(, $range) = explode('=', $_SERVER['HTTP_RANGE'], 2);
        if (strpos($range, ',') !== false) {
          header('HTTP/1.1 416 Requested Range Not Satisfiable');
          header("Content-Range: bytes $start-$end/$size");
          exit();
        }
        if ($range == '-') {
          $c_start = $size - substr($range, 1);
        } else {
          $range  = explode('-', $range);
          $c_start = $range[0];
          $c_end   = (isset($range[1]) && is_numeric($range[1])) ? $range[1] : $size;
        }

        $c_end = ($c_end > $end) ? $end : $c_end;
        if ($c_start > $c_end || $c_start > $size - 1 || $c_end >= $size) {
          header('HTTP/1.1 416 Requested Range Not Satisfiable');
          header("Content-Range: bytes $start-$end/$size");
          exit();
        }
        $start  = $c_start;
        $end    = $c_end;
        $length = $end - $start + 1;
        header('HTTP/1.1 206 Partial Content');
      }

      header("Content-Range: bytes $start-$end/$size");
      header('Content-Length: ' . $length);

      $stream = fopen($file, 'rb', false, $context);
      if (isset($_SERVER['HTTP_RANGE'])) {
        fseek($stream, $start);
      }

      $buffer = 1024 * 4;
      while (!feof($stream) && ($p = ftell($stream)) <= $end) {
        if ($p + $buffer > $end) {
          $buffer = $end - $p + 1;
        }
        set_time_limit(0);
        echo fread($stream, $buffer);
        flush();
      }

      fclose($stream);

      exit();
    }

Small files play perfectly, but larger files causes memory leak issues, specifically this line:

        fseek($stream, $start);

causes errors like the following:

PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 573177857 bytes) in /Applications/.../htdocs/.../library/aws/vendor/guzzlehttp/psr7/src/Stream.php on line 218

Which is probably:

  public function read($length)
    {
        if (!$this->readable) {
            throw new \RuntimeException('Cannot read from non-readable stream');
        }
        if ($length < 0) {
            throw new \RuntimeException('Length parameter cannot be negative');
        }

        if (0 === $length) {
            return '';
        }

        $string = fread($this->stream, $length); << this line.
        if (false === $string) {
            throw new \RuntimeException('Unable to read from stream');
        }

        return $string;
    }

[Xylane]

Hello howardlopez,

Do you have any information about what is causing this memory leak? I mean : normally in a "well written code" you clean manually what you don't use anymore in order to avoid any useless memory usage.
In AWS SDK something is not properly cleaned, but what precisely ?
Don't you have some circular reference in your code? Did you try to use php-meminfo to understand which items stay in memory?

Regarding cycle collection performance impact, ok. But why not simply getting memory limit (ini_get('memory_limit'); for example, or default value that could be customized if ini_get not available) + basic condition like "if (current memory usage + chunk size) > 95% of the memory limit, call the cycle collection", or something similar ? Anyway cycle collection must be called for bug uploads until you find the root cause, so I think it's more logical to handle this internally instead of showing how to do it manually...

[howardlopez]

@Xylane One of the key factors in the memory issues is how PHP handles closures used as a class property, which has a bug report here for creating cyclic references and is easily verified. Unfortunately, this is currently used all throughout the SDK as a key architectural component, so will likely not be able to be redesigned until the next major version.

[dinamic]

@howardlopez that's a false statement there. PHP has no issues with closures. The very same example in the bug report could be run with gc_collect_cycles() with no issues for millions of iterations.

Furthermore, as stated in the ticket you referred to, the underlying problem is PHP does not try to invoke the garbage collector upon approaching memory limits. https://bugs.php.net/bug.php?id=60982

As a permanent workaround, as that bug is a little dated (2012!!), gc_collect_cycles() could be invoked when needed.

Some of the examples posted here demonstrate the use of fread(), but they don't try to clean up the memory in any way. Maybe its a documentation issue that whomever is using the SDK in a loop should make sure to also include gc_collect_cycles() as part of that loop. Another option could be for the SDK to provide some abstraction to hide the fact.

[howardlopez]

@dinamic You are correct about gc_collect_cycles, and note that this has been referenced several times in this thread as a workaround for this issue. However, xylane has mentioned that needing to invoke the collector should ideally not be needed, but unfortunately it currently is needed with closures used in this manner, as you affirmed. Even though the bug report is dated, the behavior described for this case still exists to this day (i.e. without manually invoking the garbage collector, memory use will increase on each iteration).

Since the SDK's architecture depends on using closures in this way, it will continue to affect the SDK, and manual garbage collection will continue to be needed for certain use cases. We have documentation for some of the specific cases in which manual garbage collection may be needed, but may need to add a more general case as well.

[howardlopez]

@tedchou12 Does the $start variable in your fseek($stream, $start); line contain a value greater than your memory limit (which appears to be 128mb)? Seeking to an offset of greater than the memory limit will lead to a memory error.

You can get around this by doing multiple smaller seeks by taking advantage of the $whence parameter in the fseek function. Setting this to SEEK_CUR will start at the current position. For example, if your target offset was 250000000:

$context = stream_context_create(['s3' => ['seekable' => true]]);
$stream = fopen("s3://your-bucket/your-file", 'rb', false, $context);
$currentOffset = 0;
$step = 50000000;
$targetOffset = 250000000;
while ($currentOffset < $targetOffset) {
    $result = fseek($stream, $step, SEEK_CUR);
    $currentOffset += $step;
}

You can arrive at the target offset however you like, as long as the individual seek steps are sufficiently below the memory limit. Let us know how that works out for you.

[tedchou12]

@howardlopez
Thank you so much! You are correct! Actually, I found out that the aws-sdk-php package doesn't limit on the seek length, so if the file itself is larger than the allocated memory, the script will attempt to load the whole thing at once to the memory; which of course, causes memory leak. I have found a work around of:

        $string = fread($this->stream, min($length, 4*1024)); 

Which will force on a maximum seek of 8MB. Let me try your method, I am not so familiar with whence, but I'd love to learn it.
Thanks!

[stof]

If the issue is related to cyclic references involving closures, a solution might be to use static closures everywhere not needing to use $this inside the closure (it might not help as the issue might come from cases actually needing $this, but that's a start). Note that PHP-CS-Fixer is now able to automate this.

And if the issue is actually seeking file, the solution found by @howardlopez and @tedchou12 may be implemented in the SDK.

[howardlopez]

@stof Static closures won't be able to replace all the closures causing cyclic references (since some instances do need the object context), but we are investigating using static closures when possible.

And more generally, this issue will definitely be taken into account for the design of the next major version of the SDK.

[afagard]

I had a similar issue with this sdk. I think fixed it with gc_collect_cycles() as stated in this thread. Just wanted reaffirmation if I was using it correctly?

I followed this initially: https://docs.aws.amazon.com/AmazonS3/latest/dev/LLuploadFilePHP.html

public function addFile(File $file, $folder, $chunkSizeBytes) {
        $folder = $this->formatFolder($folder);
        $result = $this->service->createMultipartUpload([
            'Bucket' => $this->bucket,
            'Key' => $folder . $file->name,
            'StorageClass' => 'REDUCED_REDUNDANCY',
            'ACL' => $this->acl,
        ]);
        $uploadId = $result['UploadId'];
        try {
            $handle = fopen($file->pwd(), 'rb');
            $partNumber = 1;
            while (!feof($handle)) {
                $result = $this->service->uploadPart([
                    'Bucket' => $this->bucket,
                    'Key' => $folder . $file->name,
                    'UploadId' => $uploadId,
                    'PartNumber' => $partNumber,
                    'Body' => fread($handle, $chunkSizeBytes),
                ]);
                $parts['Parts'][$partNumber] = [
                    'PartNumber' => $partNumber,
                    'ETag' => $result['ETag'],
                ];
                $partNumber++;
                unset($result);
                gc_collect_cycles();
            }
            fclose($handle);
        } catch (S3Exception $e) {
            $result = $this->service->abortMultipartUpload([
                'Bucket' => $this->bucket,
                'Key' => $folder . $file->name,
                'UploadId' => $uploadId
            ]);
            throw new Exception(__CLASS__ . ": {$file->name} upload failed.");
        }
        $result = $this->service->completeMultipartUpload([
            'Bucket' => $this->bucket,
            'Key' => $folder . $file->name,
            'UploadId' => $uploadId,
            'MultipartUpload' => $parts,
        ]);
        return $file->name;
    }

Using the same way using a while loop to upload the same files to GDrive (without gc_collect_cycles()) didn't cause a memory issue.

public function addFile(File $file, $folder, $chunkSizeBytes) {
        $fileMetadata = new Google_Service_Drive_DriveFile([
            'name' => $file->name,
            'mimeType' => $file->mime(),
            'parents' => [$folder]
        ]);
        $this->client->setDefer(true);
        $service = new Google_Service_Drive($this->client);
        $request = $service->files->create($fileMetadata, [
            'uploadType' => 'resumable',
            'fields' => 'id'
        ]);
        $media = new Google_Http_MediaFileUpload(
                $this->client, $request, $file->mime(), null, true, $chunkSizeBytes
        );
        $media->setFileSize($file->size());
        $handle = fopen($file->pwd(), "rb");
        $status = false;
        while (!$status && !feof($handle)) {
            $chunk = fread($handle, $chunkSizeBytes);
            $status = $media->nextChunk($chunk);
        }
        fclose($handle);
        $this->client->setDefer(false);
        return $file->name;
    }

[howardlopez]

@afagard Calling gc_collect_cycles during loops with SDK client operations is generally the idea that's been discussed here here, and which you seem to have implemented. Garbage collection does have performance impacts, but if it matters to your use case, you can experiment and tweak how often it is called to hit the sweet spot of minimizing that impact while still not running out of memory in your scripts.