On 24 Nov 2003, at 09:59, Oliver Zeigermann wrote:
Stefano Mazzocchi wrote:
On 19 Nov 2003, at 05:03, Oliver Zeigermann wrote:Hi Stefano,Glad you asked ;-)
I am wondering why you check for any lock for the source uri. Also, why no lock for the destination? Why is there no patch for the copy method?
Could you please explain a little bit more?!
That bug is *really* subdle and was hard to find. It is basically triggered by the fact that the "move" operation performed between two different stores is not atomic, but it done as "copy" then "delete". But "copy" doesn't (nor should, IMHO) check for the lock of the source uri, but "delete" obviously does. This transforms the "move" of a locked resource a "copy", which is wrong (the litmus tests fail for this reason).
Hmmm. Do not understand... Aren't you mixing up isolation of a transaction with general locking? There are those locks set externally spanning multiple requests and there *might* be internal locks guaranteeing correctness of single requests.
I choose to fix it by checking for the lock on the source URI right away to avoid even trying to do the move if the source URI is locked. This solves the issue cleanly and it's store independent.
Hmmm, wondering where this is done for the Delete method, really could not find it. I still think the more appropriate place for this check should be MacroImpl to use this functionality from other tiers as well.
The only thing I care about is to pass the tests. I don't care how it gets fixed. I just proposed a solution that worked for me, if you think you have a better solution that fixes the problem, I'm all for it.
The offending code is in MacroImpl, in fact.
public void move(SlideToken token, String sourceUri,
String destinationUri, MacroParameters parameters,
CopyRouteRedirector copyRedirector, CopyListener copyListener,
DeleteTargetRedirector deleteRedirector, DeleteListener deleteListener)
throws CopyMacroException, DeleteMacroException {
if (Configuration.useBinding(namespace.getUri(token, sourceUri).getStore()) &&
Configuration.useBinding(namespace.getUri(token, destinationUri).getStore()) &&
sameStore(token, sourceUri, destinationUri)) {
rebind(token, sourceUri, destinationUri, parameters,
copyRedirector, copyListener, deleteRedirector, deleteListener);
using "rebind" is no problem (but for some reason this is not executed even on the same store!)
}
else {
copy(token, sourceUri, destinationUri, parameters,
copyRedirector, copyListener, deleteRedirector, deleteListener);
delete(token, sourceUri, parameters, deleteRedirector, deleteListener);
}
}the problem is here! since the first copy happens with no problems and the delete fails before of locking, leaving a orphaned copy and exiting with the wrong error code.
If you check for locking at this level (before doing the copy) it works as well, but I don't really see a reason not to check for locking in the top phase... but maybe you see something I'm missing.
-- Stefano.
smime.p7s
Description: S/MIME cryptographic signature
