menu_rebuild() / Router::rebuild() has additional race conditions which aren't accounted for by the lock.
1. Processes that fail to acquire a lock, have the menu_rebuild_needed variable / state and no menu masks in memory. If they subsequently call menu_get_item() / any function on the router that checks for rebuild, that will trigger yet another menu_rebuild() / Router::rebuild().
2. Additionally, there's a window between variable_initialize() / state() retrieval and menu_get_item() / Router::[public] where another process may have fully completed a menu_rebuild() / Router::rebuild() and freed the lock. In this case we don't need to rebuild the menu, but we do anyway.
#1 is a serious race condition - it can cause menu_rebuild() / Router::rebuild() to occur over and over again - as one process acquires the lock, other processes lock_wait() / Lock::wait(), then rebuild, then acquire the lock, then more process lock_wait() / Lock::wait() etc. - proper lock stampede.
#2 is more of an optimization to make the current stampede protection more robust.
Both problems exist in 8.x too despite the code having changed a lot. They have been shown in above code with the / for D8 as the alternative implementation.
Its really just the naming that has changed.
Move the logic away from a lazy-rebuild model into a explicit rebuild model, which means, there is no global (across requests) flag, to trigger a rebuild.
Instead the router is marked as to be rebuild only in the current process. At the end of the request ($kernel->terminate()) the rebuild is triggered.
That way we have an important performance improvement (or at least avoiding regressions) with this patch:
- No more lock stampede where multiple requests try to rebuild routes (this reduces problems of things like
This leads to the following additional results:
- A Drupal standard installation will be 30% slower since multiple route rebuilds are triggered that are not necessary. See the follow-up to address this.
(done #203) Agree that the performance improvements are worth it.
User interface changes
RouteBuilderIndicator + Interface + service is removed.
Everytime menu_rebuild() is called, we get a crazy amount of waits on INSERT INTO menu_router and a bunch of DELETE FROM menu_router. I believe there is some sort of race condition for rebuilding menus. The only way I can kill it is to kill off a bunch of processes. I would imagine that setting a lock for the first user to rebuild that table would be a simple enough fix, but I wanted to open a ticket and see if anyone else had come up with a solution first.
Please help, this is crashing our site: http://www.divx.com/ every time we update a menu or clear the cache.
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,982 pass(es). View
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 356399_2.patch. Unable to apply patch. See the log in the details link for more information. View
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,916 pass(es). View
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,615 pass(es). View