mirror of
https://github.com/doctrine/orm.git
synced 2026-04-29 09:23:20 +02:00
DDC-136: Usage of spl_object_hash in UnitOfWork is dangerous #168
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @doctrinebot on GitHub (Nov 11, 2009).
Jira issue originally created by user nicokaiser:
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...
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by romanb:
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.
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by romanb:
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.
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by nicokaiser:
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)
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by @beberlei:
Btw, why dont we use SplObjectStorage at this point? Since 5.3 it has some (undocumented) behaviour that goes like:
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().
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by romanb:
@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!
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by romanb:
@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.
@doctrinebot commented on GitHub (Nov 11, 2009):
Issue was closed with resolution "Fixed"
@doctrinebot commented on GitHub (Nov 11, 2009):
Comment created by @beberlei:
@Roman: the reference issue is a serious drawback of SplObjectStorage, good that the bug could be fixed with spl_object_hash nonetheless. :-)
@doctrinebot commented on GitHub (Dec 27, 2011):
Comment created by gamesh: