I had two admins (me and another "able-bodied" person) logged in at the same time modifying blocks when all of a sudden all of our blocks that we had enabled (save the default enabled blocks) were disabled and most of the settings (visibility, etc.) were gone.

This is the only time this has happened to me and coincidentally the only time I've had two admins in there at the same exact time.

I looked at _block_rehash() where there is a SELECT, DELETE and INSERT. Is this possible that after I called my DELETE, she called her SELECT (selecting nothing), then after my INSERT, she called her DELETE (deleting my changes), then called her INSERT (inserting nothing)?

Should there be a LOCK TABLES here to prevent this freakishness and make this a db transaction? Also, should we always do a LOCK TABLES when doing this type of SELECT, DELETE, INSERT?

CommentFileSizeAuthor
#11 block.module.HEAD.patch1.77 KBRobRoy
#7 block.module_17.patch1.76 KBRobRoy
#5 block.module_16.patch1.89 KBRobRoy
#2 block.module_15.patch1.83 KBRobRoy

Comments

RobRoy’s picture

RobRoy commented

chx pointed me to this issue http://drupal.org/node/55516 which deals with a db-independent REPLACE.

RobRoy’s picture

RobRoy commented
Status: Active » Needs review
StatusFileSize
new block.module_15.patch1.83 KB

I think this may fix the critical data loss issue.

@chx Can you confirm this logic?

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

Please don't abuse the database API and use more normal functions than '_db_query_callback()'

quicksketch’s picture

quicksketch
he/him
commented

Confirmation of this report. I was having a hard time replicating the sudden loss of custom display logic, but I knew the problem originated in _block_rehash since it's the only place where the display logic is modified. Good work on tracking this down.

RobRoy’s picture

RobRoy commented
Status: Needs work » Needs review
StatusFileSize
new block.module_16.patch1.89 KB

A similar approach is used in devel and views. I guess I was just trying to follow a backport of what was being done in http://drupal.org/node/55516.

But I agree, it is not really needed here. Check out this new patch with a cleaner method.

chx’s picture

chx commented
Status: Needs review » Needs work

this is mysql specific code. and yes, it's butt-ugly to begin with.

RobRoy’s picture

RobRoy commented
Status: Needs work » Needs review
StatusFileSize
new block.module_17.patch1.76 KB

A new patch, hopefully less butt-ugly and MySQL-specific.

RobRoy’s picture

RobRoy commented

Bumping...IMHO this is a big, big issue and the latest patch is a solid fix that needs to get into DRUPAL-4-7.

If two admins access admin > blocks at perfectly the same time, block settings will be RESET to defaults, clearing all blocks! I'm not talking about two admins saving block settings at the same time, I'm talking about simply accessing the admin/block path at the same time! Imagine this on a production site where two admins are just viewing that blocks page and all the blocks on a live server are removed. Nasty.

Please, can I get holla from the back? :D

killes@www.drop.org’s picture

killes@www.drop.org commented

looks ok to me.

m3avrck’s picture

m3avrck commented

Ran into this problem on a major site with lots of blocks... please commit not only to HEAD but also backport to 4.7 too! Thanks!

RobRoy’s picture

RobRoy commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new block.module.HEAD.patch1.77 KB

The last patch was for DRUPAL-4-7 and this one is for HEAD. Marking this RTBC, but please let me know if that is out of line.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

killes@www.drop.org’s picture

killes@www.drop.org commented

also to 4.7

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)