This is an automated email from the git hooks/post-receive script.

Git pushed a commit to branch master
in repository ffmpeg.

commit 3d8488bed763e01e5d833cd253333ee6c0e4b010
Author:     Niklas Haas <[email protected]>
AuthorDate: Sun Jun 14 11:21:19 2026 +0200
Commit:     Niklas Haas <[email protected]>
CommitDate: Mon Aug 31 19:10:45 2026 +0000

    avformat/shared: clean up PENDING state on error/early exit
    
    This makes sure threads that fail for reasons other than the underlying I/O
    failing clean up after their own PENDING state on failure, unless another
    thread updated the block state in the meantime.
    
    Subsumes the existing "is_race" condition, which is inverted to "acquired"
    that is 1 exactly when we were the thread that set the PENDING state.
    
    Sponsored-by: nxtedition AB
    Signed-off-by: Niklas Haas <[email protected]>
---
 libavformat/shared.c | 56 +++++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/libavformat/shared.c b/libavformat/shared.c
index 3e24dabab4..eba640c207 100644
--- a/libavformat/shared.c
+++ b/libavformat/shared.c
@@ -613,7 +613,7 @@ static int shared_read(URLContext *h, unsigned char *buf, 
int size)
     Block *const block = &s->spacemap->blocks[block_id];
     unsigned state = atomic_load_explicit(&block->state, memory_order_acquire);
     int64_t pending_since = 0;
-    int verify_read = 0, is_race = 0;
+    int verify_read = 0, acquired = 0;
 
 retry:
     switch (state) {
@@ -689,6 +689,7 @@ read_block:
                                                     memory_order_acquire))
         {
             /* Acquired pending state, proceed to fetch the block */
+            acquired = 1;
             state = BLOCK_PENDING;
             break;
         }
@@ -698,14 +699,11 @@ read_block:
     case BLOCK_PENDING:
         /* Another thread is busy fetching this block, wait for it to finish */
         if (!s->timeout) {
-            is_race = 1;
             break; /* no timeout requested, immediately race to fetch block */
         } else if (pending_since) {
             int64_t new = av_gettime_relative();
-            if (new - pending_since >= s->timeout) {
-                is_race = 1;
+            if (new - pending_since >= s->timeout)
                 break; /* timeout expired, try to fetch the block ourselves */
-            }
         } else {
             pending_since = av_gettime_relative();
         }
@@ -716,6 +714,17 @@ read_block:
         goto retry;
     }
 
+    /* Release pending state on failure to avoid stalling other threads */
+#define RELEASE_PENDING(block, state)                                          
\
+    do {                                                                       
\
+        if (acquired) {                                                        
\
+            av_assert1(state == BLOCK_PENDING);                                
\
+            atomic_compare_exchange_strong_explicit(                           
\
+                &block->state, &state, BLOCK_NONE, memory_order_relaxed,       
\
+                memory_order_relaxed);                                         
\
+        }                                                                      
\
+    } while (0)
+
     /* Cache miss, fetch this block from underlying protocol */
     s->nb_miss++;
 
@@ -726,15 +735,7 @@ read_block:
         if (inner_pos < 0) {
             av_log(h, AV_LOG_ERROR, "Failed to seek underlying protocol: %s\n",
                    av_err2str(inner_pos));
-            if (!read_only) {
-                /* Release pending state to avoid stalling other threads. Don't
-                 * mark this as failed, since the seek error may be unrelated 
to
-                 * the block and should probably be tried again. */
-                atomic_compare_exchange_strong_explicit(&block->state, &state,
-                                                        BLOCK_NONE,
-                                                        memory_order_relaxed,
-                                                        memory_order_relaxed);
-            }
+            RELEASE_PENDING(block, state);
             return inner_pos;
         }
 
@@ -745,8 +746,10 @@ read_block:
     if (read_only) {
         /* Directly defer to the underlying protocol */
         ret = ffurl_read(s->inner, buf, size);
-        if (ret < 0)
+        if (ret < 0) {
+            av_assert1(!acquired);
             return ret;
+        }
 
         /* Verify the read data against the cached data if requested */
         if (verify_read && memcmp(buf, tmp, ret)) {
@@ -760,7 +763,7 @@ read_block:
     }
 
     int write_back = 1;
-    if (s->cache_data && !is_race) {
+    if (s->cache_data && acquired) {
         /* Read directly into memory mapped cache file */
         tmp = s->cache_data + block_pos;
         write_back = 0;
@@ -782,15 +785,16 @@ read_block:
         else if (ret < 0) {
             av_log(h, AV_LOG_ERROR, "Failed to read block 0x%"PRIx64": %s\n",
                    block_id, av_err2str(ret));
-            int new_state = BLOCK_FAILED;
-            if (ret == AVERROR(EAGAIN) || ret == AVERROR_EXIT)
-                new_state = BLOCK_NONE; /* transient error, allow retries */
+            if (ret == AVERROR(EAGAIN) || ret == AVERROR_EXIT) {
+                RELEASE_PENDING(block, state);
+                return ret; /* transient error, allow retries */
+            }
 
             /* Try to mark block as failed; ignore errors - any mismatch
              * here will mean that either another thread already marked it
              * as failed, or successfully cached it in the meantime */
             atomic_compare_exchange_strong_explicit(&block->state, &state,
-                                                    new_state,
+                                                    BLOCK_FAILED,
                                                     memory_order_relaxed,
                                                     memory_order_relaxed);
             return ret;
@@ -803,8 +807,10 @@ read_block:
     if (bytes_read < block_size) {
         /* Learned location of true EOF, update filesize */
         ret = set_filesize(h, inner_pos + bytes_read);
-        if (ret < 0)
+        if (ret < 0) {
+            RELEASE_PENDING(block, state);
             return ret;
+        }
     }
 
     if (bytes_read > 0) {
@@ -813,12 +819,7 @@ read_block:
             av_log(h, AV_LOG_ERROR, "Failed to write to cache file: %s\n",
                    av_err2str(ret));
             s->write_err = 1;
-            /* Mark as NONE, not FAILED, since the block itself is fine -
-             * just absent from the cache. */
-            atomic_compare_exchange_strong_explicit(&block->state, &state,
-                                                    BLOCK_NONE,
-                                                    memory_order_relaxed,
-                                                    memory_order_relaxed);
+            RELEASE_PENDING(block, state);
         } else {
             uint32_t crc = get_block_crc(tmp, bytes_read);
             av_log(h, AV_LOG_TRACE, "Cached %d bytes to block 0x%"PRIx64" at "
@@ -827,6 +828,7 @@ read_block:
             atomic_store_explicit(&block->state, crc, memory_order_release);
         }
     } else {
+        RELEASE_PENDING(block, state);
         return AVERROR_EOF;
     }
 

-- 
To stop receiving notification emails like this one, please contact
[email protected].
_______________________________________________
ffmpeg-cvslog mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to