Raw Generator in LazyCollection::make() is not lazy

[sebsel]

• Laravel Version: 7.15.0
• PHP Version: 7.4.0

Description:

When using Lazy Collections, one can use the following to read a large file without loading it all in memory:

use Illuminate\Support\LazyCollection;

LazyCollection::make(function () {
    $handle = fopen('file.txt', 'r');
    while ($line = fgets($handle)) yield $line;
})->map(...);

However, if you have a raw Generator (the result of calling a Generator function), then the LazyCollection will not handle it lazy anymore. The following will crash on out of memory when the file is too large:

use Illuminate\Support\LazyCollection;

class FileReader
{
    public function read($file)
    {
        $handle = fopen($file, 'r');
        while ($line = fgets($handle)) yield $line;
    }
}

$reader = new FileReader();

LazyCollection::make($reader->read('file.txt'))->map(...);

The solution is to change it to the following:

LazyCollection::make(fn() => yield from $reader->read('file.txt'))->map(...);

It cost me some time to figure out this bug, and it surprised me: I think Laravel can handle this for me?

How it works

LazyCollection::make() calls the constructor, which calls getArrayableItems() from the EnumeratesValues trait.

framework/src/Illuminate/Support/LazyCollection.php

Lines 29 to 39 in a00484a

	public function __construct($source = null)
	{
	if ($source instanceof Closure || $source instanceof self) {
	$this->source = $source;
	} elseif (is_null($source)) {
	$this->source = static::empty();
	} else {
	$this->source = $this->getArrayableItems($source);
	}
	}

The generator falls into the case that checks for Traversable, which makes that Laravel calls iterator_to_array() on it. This will run the full generator, and thus in my example load the full file in memory.

framework/src/Illuminate/Support/Traits/EnumeratesValues.php

Lines 870 to 887 in a00484a

	protected function getArrayableItems($items)
	{
	if (is_array($items)) {
	return $items;
	} elseif ($items instanceof Enumerable) {
	return $items->all();
	} elseif ($items instanceof Arrayable) {
	return $items->toArray();
	} elseif ($items instanceof Jsonable) {
	return json_decode($items->toJson(), true);
	} elseif ($items instanceof JsonSerializable) {
	return (array) $items->jsonSerialize();
	} elseif ($items instanceof Traversable) {
	return iterator_to_array($items);
	}
	return (array) $items;
	}

A solution could be to change the constructor of LazyCollection to the following:

    public function __construct($source = null)
    {
        if ($source instanceof Closure || $source instanceof self) {
            $this->source = $source;
        } elseif ($source instanceof Generator) {
            $this->source = function () use ($source) {
                yield from $source;
            };
        } elseif (is_null($source)) {
            $this->source = static::empty();
        } else {
            $this->source = $this->getArrayableItems($source);
        }
    }

[driesvints]

Heya. This sounds like something more for discussions so I'm moving it there.

[driesvints]

Also pinging @JosephSilber since he probably has some insights here.

[JosephSilber]

The documentation does not say nor suggest that you can pass a Generator to the LazyCollection's make method.

But this keeps on coming up every now and then, so maybe we should document it properly somewhere.

---

I think Laravel can handle this for me?

I don't think it can, since iterators (AKA Generators) aren't rewindable.

---

With your proposed solution:

public function __construct($source = null)
{
    if ($source instanceof Generator) {
        $this->source = function () use ($source) {
            yield from $source;
        };
    }
    // other options omitted for brevity...
}

...this code will break:

$collection->first() == $collection->first(); // false :(