Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-136

Usage of spl_object_hash in UnitOfWork is dangerous

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-ALPHA2
    • Fix Version/s: 2.0-ALPHA3
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      UnitOfWork uses spl_object_hash to identify objects. This breaks horribly as spl_object_hash is not unique when using it in different scopes.

      See this example:
      http://pastebin.com/d5e4825b0

      When creating and destroying (persisting and removing) objects in sub functions, UnitOfWork gets confused.
      I don't think this is a special case, so I consider this as a Blocker...

        Activity

        Hide
        Roman S. Borschel added a comment -

        Of course we know how spl_object_hash works and we dont see any problems with it with regards to Doctrine. There is never the same object hash on two different objects that are alive at the same time. Doctrine keeps references to all objects its manages. If they get detached then Doctrine does not care about them.

        Please show an example where the spl_object_hash behavior, or more concretely, the fact that hashes from destroyed objects are reused, is a problem for Doctrine.

        Show
        Roman S. Borschel added a comment - Of course we know how spl_object_hash works and we dont see any problems with it with regards to Doctrine. There is never the same object hash on two different objects that are alive at the same time. Doctrine keeps references to all objects its manages. If they get detached then Doctrine does not care about them. Please show an example where the spl_object_hash behavior, or more concretely, the fact that hashes from destroyed objects are reused, is a problem for Doctrine.
        Hide
        Roman S. Borschel added a comment -

        To clarify, there might be (a) bug(s) where the UnitOfWork wrongly keeps entries in its internal arrays with the spl_object_hash of an object it does no longer care about. But this is then rather a bug in our code and not a problem that is inherent to the behavior of spl_object_hash.

        As long as Doctrine (and the UnitOfWork) cares about an object, it keeps a reference to it, hence the hash is not reused by any new objects. When Doctrine no longer cares bout an object, all entries of that object should be wiped from the UnitOfWork internal arrays and then it does not matter that later a new object gets the same hash again.

        I hope I explained that well.

        Thanks for your help.

        Show
        Roman S. Borschel added a comment - To clarify, there might be (a) bug(s) where the UnitOfWork wrongly keeps entries in its internal arrays with the spl_object_hash of an object it does no longer care about. But this is then rather a bug in our code and not a problem that is inherent to the behavior of spl_object_hash. As long as Doctrine (and the UnitOfWork) cares about an object, it keeps a reference to it, hence the hash is not reused by any new objects. When Doctrine no longer cares bout an object, all entries of that object should be wiped from the UnitOfWork internal arrays and then it does not matter that later a new object gets the same hash again. I hope I explained that well. Thanks for your help.
        Hide
        Nico Kaiser added a comment -

        Ok, you are right, but then UnitOfWork keeps internal entries when it shouldn't.

        Example:
        http://pastebin.com/d71a638f8

        (The both User objects in the highlighted linesget the same object hashes, the last pesist() thinks the object is detached and fails)

        Show
        Nico Kaiser added a comment - Ok, you are right, but then UnitOfWork keeps internal entries when it shouldn't. Example: http://pastebin.com/d71a638f8 (The both User objects in the highlighted linesget the same object hashes, the last pesist() thinks the object is detached and fails)
        Hide
        Benjamin Eberlei added a comment -

        Btw, why dont we use SplObjectStorage at this point? Since 5.3 it has some (undocumented) behaviour that goes like:

        $entity = new Entity();
        $storage = new SplObjectStorage()
        $storage[$entity] = array('foo' => 'bar');
        
        $foo = $storage[$entity]['foo'];
        

        This might save some additional performance, because I think SplObjectStorage uses the PHP internal object pointer and does not need to calculate the spl_object_hash().

        Show
        Benjamin Eberlei added a comment - Btw, why dont we use SplObjectStorage at this point? Since 5.3 it has some (undocumented) behaviour that goes like: $entity = new Entity(); $storage = new SplObjectStorage() $storage[$entity] = array('foo' => 'bar'); $foo = $storage[$entity]['foo']; This might save some additional performance, because I think SplObjectStorage uses the PHP internal object pointer and does not need to calculate the spl_object_hash().
        Hide
        Roman S. Borschel added a comment -

        @Nico: Thanks for the example, I will look into it.

        @Benjamin: I've played with the idea of SplObjectStorage several times already. It has some disadvantages. First, contrary to your and my first belief it is slower, not faster than using an array with spl_object_hash. Secondly it results in additional references to the objects that you need to be aware of and can potentially prevent the objects from being garbage collected.

        I mean if I store some associated information to an entity with the spl_object_hash => value information there is no reference to the actual object. Using SplObjectStorage would result in object references being stored. I've always tried to keep the locations where Doctrine keeps references to objects to a minimum to not provoke unnecessary potential memory leaks.

        If you would like to make your own performance tests of SplObjectStorage vs array + spl_object_hash I would be glad to see your results!

        Show
        Roman S. Borschel added a comment - @Nico: Thanks for the example, I will look into it. @Benjamin: I've played with the idea of SplObjectStorage several times already. It has some disadvantages. First, contrary to your and my first belief it is slower, not faster than using an array with spl_object_hash. Secondly it results in additional references to the objects that you need to be aware of and can potentially prevent the objects from being garbage collected. I mean if I store some associated information to an entity with the spl_object_hash => value information there is no reference to the actual object. Using SplObjectStorage would result in object references being stored. I've always tried to keep the locations where Doctrine keeps references to objects to a minimum to not provoke unnecessary potential memory leaks. If you would like to make your own performance tests of SplObjectStorage vs array + spl_object_hash I would be glad to see your results!
        Hide
        Roman S. Borschel added a comment -

        @Nico: The issue you mentioned should be fixed now. I reproduced it in the sandbox as well as in the tests (with some effort). So we have test coverage for this case as well at least. If you find any more of such things feel free to open new issues.

        Thanks for your help on improving Doctrine.

        Show
        Roman S. Borschel added a comment - @Nico: The issue you mentioned should be fixed now. I reproduced it in the sandbox as well as in the tests (with some effort). So we have test coverage for this case as well at least. If you find any more of such things feel free to open new issues. Thanks for your help on improving Doctrine.
        Hide
        Benjamin Eberlei added a comment -

        @Roman: the reference issue is a serious drawback of SplObjectStorage, good that the bug could be fixed with spl_object_hash nonetheless.

        Show
        Benjamin Eberlei added a comment - @Roman: the reference issue is a serious drawback of SplObjectStorage, good that the bug could be fixed with spl_object_hash nonetheless.
        Hide
        Justinas added a comment -

        I've played with the idea of SplObjectStorage several times already. It has some disadvantages. First, contrary to your and my first belief it is slower, not faster than using an array with spl_object_hash
        @Roman: how did you come to that conclusion? because there was a performance test made, and it showed that SplObjectStorage was faster and consumed less memory than php native array
        here is the link to the benchmark, if interested
        http://technosophos.com/content/set-objects-php-arrays-vs-splobjectstorage

        Show
        Justinas added a comment - I've played with the idea of SplObjectStorage several times already. It has some disadvantages. First, contrary to your and my first belief it is slower, not faster than using an array with spl_object_hash @Roman: how did you come to that conclusion? because there was a performance test made, and it showed that SplObjectStorage was faster and consumed less memory than php native array here is the link to the benchmark, if interested http://technosophos.com/content/set-objects-php-arrays-vs-splobjectstorage

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Nico Kaiser
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: