To keep the internal book-keeping in synch with the reality reflected in the database the lock_may_be_available routine should unset($locks[$name]); before exiting. This is now consistent with the behaviour in lock_acquire when it detects a broken lock.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | lock_may_be_available-806708-4.patch | 1.6 KB | superspring |
Comments
Comment #1
damien tournoud commentedlock_may_be_available()assumes that the caller doesn't already have the lock (as hinted in the function documentation). That said, it cannot hurt. Care to roll a patch?Comment #2
lostchord commentedDocumentation should never make 'hints', that's just a cover for sloppy documentation.
I'll get round to rolling patches when I've finished wandering through core. I'm just picking a few interesting bits at present and the hit rate is a bit alarming.
Edit:
I'm a relative newb (very, in fact) to PHP. My background has a large amount of assembler in it. One thing I'm working through at present, and I'd like to get sorted before 'rolling patches', is the best way to return success/failure AND details of the success/failure. My old-time VAX experience makes me love the 'lower bit set is success, lower bit clear is failure' pattern with the exact value (all the other bits) providing more detail that you can make decisions on. So, for example, I can execute a SELECT query and have it fail and pass that back (it failed) along with additional details like "nothing returned", or "database connection failed", or..... Some standardised way of doing this in all situations would allow far more robust code to be developed without the need to do really detailed research into the specifics of every call being made. In this context documentation is VERY specific about what you can do and what you can expect the results to be. This is a nice example (one of many) in the documentation set here.
My take on this little scan I'm doing at the moment is that 'Core should be absolutely bullet proof'. That means protecting against sloppy coding now. It also means protecting against well meaning Core changes that are done in good faith at some later date that might break todays code. It's basic defensive programming, and 40 years of experience has given me plenty of opportunities to see it pay off big time.
cheers
Comment #3
NROTC_Webmaster commentedI'm not very familiar with this function but my understanding is that you want to return true if it breaks a lock or if the lock is not set. In keeping with this you would not want to unset the lock when returning FALSE. The only way I could come up with a solution for this is the following. Again though since I'm not familiar with this look this function over and let me know if this is what you were looking for. I can't really imagine a scenario where this would be necessary but if it is what you were looking for I can make the patch.
Comment #4
superspring commentedAs described above.
Comment #18
quietone commented@lostchord, Thank you for reporting this problem. We rely on issue reports like this one to resolve bugs and improve Drupal core.
There has been no activity here for 10 years. Is this change still desired?
Since we need more information to move forward with this issue, I am setting the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #19
quietone commentedI asked in #bugsmash about this. larowlan responded that this can be closed because it is referring to global $locks which is not used anymore.
Therefor closing as outdated.