Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Follow-up to #251792: Implement a locking framework for long operations, we want to use locks to prevent concurrent image generation rather than relying on Drupal's cache which are potentially prone to race conditions due to caching.
Comment | File | Size | Author |
---|---|---|---|
#7 | image-lock-2.patch | 6.17 KB | c960657 |
#1 | image-lock-1.patch | 5.83 KB | c960657 |
Comments
Comment #1
c960657 CreditAttribution: c960657 commentedComment #2
drewish CreditAttribution: drewish commentedI'm not sure if we really get the win here because we're still using cache_image for granting access to temp images.
Comment #3
c960657 CreditAttribution: c960657 commentedWhat do you mean by “win”? cache_image is used for two unrelated purposes. One of them is a kind of semaphore, but one that doesn't address race conditions like the locking framework does.
Comment #4
drewish CreditAttribution: drewish commentedthe ironic thing is that it's used for two unrelated tasks—neither of which is caching...
Comment #5
drewish CreditAttribution: drewish commentedignore my attempt to derail this. it's rtbc. we can sort out the right way for image to do it's permissions later.
Comment #6
Dries CreditAttribution: Dries commentedLet's write
+ if (!file_exists($destination) && !($lock_acquired = lock_acquire($lock_name))) {
in two lines. Set RTBC after. Thanks!Comment #7
c960657 CreditAttribution: c960657 commentedI made it into two if statements to make it even clearer that
=
is an assignment, not a comparison, and that lock_acquire() is only called when file_exists() returns false.Comment #8
webchickLooks good! Committed to HEAD. Thanks!