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

$set->toArray is required for Iterator #82

Closed
basz opened this issue Mar 27, 2017 · 6 comments
Closed

$set->toArray is required for Iterator #82

basz opened this issue Mar 27, 2017 · 6 comments

Comments

@basz
Copy link

basz commented Mar 27, 2017

with the extension v1.1.8 (polypill is fine) I seeing this strange behaviour using php 7.1.3

#!/usr/bin/env php
<?php

declare(strict_types=1);

class RolesWhereUpdated
{
    private $additions;

    public static function withChanges(
        \Ds\Set $additions
    ): RolesWhereUpdated {
        $event = new static();

        $event->additions = $additions->toArray();

        return $event;
    }

    public function additions(): \Ds\Set
    {
        return new \Ds\Set($this->additions);
    }
}

$event = RolesWhereUpdated::withChanges(new \Ds\Set(['abc', 'def']));

// not iterated
foreach ($event->additions() as $item => $value) {
    print 'iterated: $event->additions()' . PHP_EOL;
}

// is iterated
foreach ($event->additions()->toArray() as $item => $value) {
    print 'iterated: $event->additions()->toArray()' . PHP_EOL;
}


// but with a plain set
$set = new \Ds\Set(['abc', 'def']);

// this is iterated
foreach ($set as $item => $value) {
    print 'iterated: $set' . PHP_EOL;
}

expected output

iterated: $event->additions()
iterated: $event->additions()
iterated: $event->additions()->toArray()
iterated: $event->additions()->toArray()
iterated: $set
iterated: $set

actual output

iterated: $event->additions()->toArray()
iterated: $event->additions()->toArray()
iterated: $set
iterated: $set
@nikic
Copy link
Contributor

nikic commented Mar 27, 2017

The problem is presumably that https://github.com/php-ds/extension/blob/master/src/php/iterators/php_htable_iterator.c#L160 does not retain a reference to the object, so in this case the object will be destroyed while the iterator is still in use.

@basz
Copy link
Author

basz commented Mar 27, 2017

by the way, the same is true for Map

@rtheunissen
Copy link
Member

@nikic coming back to this, what would be the best way to solve this? I get that we need to hold on to a reference of the object so that it doesn't get collected, so should we add that reference when we create the iterator and release it when we destruct the iterator?

@rtheunissen
Copy link
Member

@nikic see #84

@rtheunissen
Copy link
Member

Can confirm that the fix in #84 produces:

iterated: $event->additions()
iterated: $event->additions()
iterated: $event->additions()->toArray()
iterated: $event->additions()->toArray()
iterated: $set
iterated: $set

@rtheunissen
Copy link
Member

This has now been fixed and released as v1.1.9

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

No branches or pull requests

3 participants