DatabaseCacheBackend::ensureBinExists() does not properly handle exceptions

I recommend putting the exception text in the issue summary so someone may find this. Contributors are busy and may not look at screenshots.

Also I recommend retitling this to “Cache clear occasionally throws” followed by a condensed bit of the exception message.

The class containing ensureBinExists was last changed in December 2017.

I find it interesting that ensureBinExists tries to catch a SchemaObjectExistsException however Connection::handleQueryException will never produce a SchemaObjectExistsException.

The cache clearing codepath executes "TRUNCATE TABLE" on each cache table. "Truncate operations drop and re-create the table, which is much faster than deleting rows one by one, particularly for large tables", according to the MySQL docs.

So I think that ensureBinExists isn't performing as intended because it cannot catch the correct exception in the event another process has already created the table.

#3046962: Race condition causing 500 on cache table creation was duplicate.

@catch
Thank you for finding the duplicated issue.
It seems there are two things we can do:

• Try to be able to throw the exception by parsing the exception somehow. I would expect that this could be tricky to get right
• We could catch all database exceptions when creating the cache bin (and potentially all the other lazy created tables).

I've implemented the second option as it seems to be pragmatic.

Agreed with catching the less specific exception, it could potentially lead to silently not creating the table, but I cant really think of a situation where that would happen permanently and not affect anything else that would log/show an error.

That seems like a work-around though...? Can we reproduce this in a test? We should handle this and not gracefully ignore it IMO. Lock mechanism seems more valid to me.

That seems like a work-around though...? Can we reproduce this in a test? We should handle this and not gracefully ignore it IMO. Lock mechanism seems more valid to me.

it is really hard to write reliable/useful tests for race conditions. I can't think of a good one, but I might miss something.

I've discussed the solution with @dawehner and I agree with @catch that this seems like the best solution. I don't think we can use the lock system because the lock system is affected by this :)

I've checked that all code that does this on demand table creation is fixed and it appears to be. I think we should file a non-blocking follow-up to try to move some of this auto table creation logic to a trait.

I found the followup already in the issue queue :) #2721271: Simplify exception handling for lazy-load pattern

it is really hard to write reliable/useful tests for race conditions. I can't think of a good one, but I might miss something.

Yeah, not sure. OWASP suggests black box testing. not sure if it's ever been done in Drupal, but it's definitely a interesting approach to see if more unexpected behaviour can be found by concurrency issues.

@johndevman I'd be interested in seeing whether this is actually possible. That would be cool.

We could write a test that manually throws the exception that should have been caught. Though that would again lead to a false sense of safety.

Given that, I think we can move forward with the current patch.

Updating credit...

Removing some unused uses.

FILE: ...s/drupal8alt.dev/core/lib/Drupal/Core/Config/DatabaseStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 8 | WARNING | [x] Unused use statement
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 72ms; Memory: 8MB

PHPCS: core/lib/Drupal/Core/Flood/DatabaseBackend.php passed
PHPCS: core/lib/Drupal/Core/Lock/DatabaseLockBackend.php passed

FILE: ...tes/drupal8alt.dev/core/lib/Drupal/Core/Menu/MenuTreeStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 14 | WARNING | [x] Unused use statement
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 172ms; Memory: 16MB

PHPCS: core/lib/Drupal/Core/Path/AliasStorage.php passed
PHPCS: core/lib/Drupal/Core/Queue/DatabaseQueue.php passed

FILE: ...es/drupal8alt.dev/core/lib/Drupal/Core/Routing/MatcherDumper.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 6 | WARNING | [x] Unused use statement
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Unrelated Javascript test fail.

Committed/pushed to 8.8.x and cherry-picked to 8.7.x, thanks!