[CsvEncoder] Impossible to use with Traversable #60141
Labels
Comments
|
not working with generators (which can only be iterated once) is not the same than not working with traversable. I think only generators are broken. |
|
The foreach by reference and the multiple iterations over the same iterable make it impossible to use the CsvEncoder with Here is a reproducer: /** @dataProvider provideIterable */
public function testIterable(mixed $data)
{
$this->assertEquals(<<<'CSV'
foo,bar
hello,"hey ho"
hi,"let's go"
CSV, $this->encoder->encode($data, 'csv'));
}
public static function provideIterable()
{
$data = [
['foo' => 'hello', 'bar' => 'hey ho'],
['foo' => 'hi', 'bar' => 'let\'s go'],
];
// OK
yield 'array' => [$data];
// OK
yield 'array iterator' => [new \ArrayIterator($data)];
// Error: An iterator cannot be used with foreach by reference
yield 'iterator aggregate' => [new \IteratorIterator(new \ArrayIterator($data))];
// Exception: Cannot traverse an already closed generator
yield 'generator' => [(fn (): \Generator => yield from $data)()];
}A solution is to convert the Generator or Iterator with if ($data instanceof \Traversable) {
// Exhaust the iterator so that we can rewind it later and access values by-reference
$data = iterator_to_array($data);
}Edit: updated to make the dataprovider static. |
fabpot
added a commit
that referenced
this issue
May 12, 2025
…nerator` error by materializing Traversable input (santysisi) This PR was merged into the 6.4 branch. Discussion ---------- [Serializer] Prevent `Cannot traverse an already closed generator` error by materializing Traversable input | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #60141 | License | MIT ### ✅ Pull Request description: This PR addresses the issue reported in the linked ticket, specifically the error: `Cannot traverse an already closed generator` The fix involves converting `Traversable` input into an array using `iterator_to_array($data)`, in order to allow multiple traversals of generator-based inputs. At first glance, it might seem that this approach significantly increases memory usage, since all generator values are stored in memory. However, this is not the case. In the current logic, the following [loop](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Serializer/Encoder/CsvEncoder.php#L82-L86) already forces all values from the generator into memory: ```php foreach ($data as &$value) { $flattened = []; $this->flatten($value, $flattened, $keySeparator, '', $escapeFormulas); $value = $flattened; } ``` Therefore, materializing the generator with `iterator_to_array() `does not increase peak memory usage in any meaningful way. As an example, here's the comparison of peak memory usage (measured with [memory_get_peak_usage](https://www.php.net/manual/en/function.memory-get-peak-usage.php)) when processing an array of 1 million integers: * **With the fix**: 90,044,272 bytes * **Without the fix**: 89,936,680 bytes The difference is negligible, confirming that the fix does not introduce a meaningful performance cost in terms of memory. Commits ------- c7206a9 Fix: prevent "Cannot traverse an already closed generator" error by materializing Traversable input
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Symfony version(s) affected
7.2
Description
CsvEncoder accepts iterator to
encodefunction, but it doesn't work properly with Traversable or any non-simple-array iteratorsHow to reproduce
I get an exception
Cannot traverse an already closed generatorPossible Solution
No response
Additional Context
No response
The text was updated successfully, but these errors were encountered: