diff mbox

[FFmpeg-devel] lavc/utils: simplify lockmgr

Message ID 20171127044419.20562-1-atomnuker@gmail.com
State New
Headers show

Commit Message

Rostislav Pehlivanov Nov. 27, 2017, 4:44 a.m. UTC
Again, totally unneded use of the atomic function to set/NULL a local variable.

Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
---
 libavcodec/utils.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Michael Niedermayer Nov. 28, 2017, 2:03 a.m. UTC | #1
On Mon, Nov 27, 2017 at 04:44:19AM +0000, Rostislav Pehlivanov wrote:
> Again, totally unneded use of the atomic function to set/NULL a local variable.
> 
> Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
> ---
>  libavcodec/utils.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

this crashs ffplay

ffplay_g && gdb --args ./ffplay_g tests/data/fate/vsynth1-snow.avi

#0  0x000000000106f8a3 in av_freep ()
#1  0x0000000000b103c4 in default_lockmgr_cb ()
#2  0x0000000000b13dd3 in av_lockmgr_register ()
#3  0x00000000004ab00d in main ()

if you cant reproduce, say so, and ill rebuild and produce a better
backtrace

[...]
Timothy Gu Nov. 30, 2017, 7:34 a.m. UTC | #2
On Sun, Nov 26, 2017 at 8:44 PM Rostislav Pehlivanov <atomnuker@gmail.com>
wrote:

> @@ -103,8 +104,7 @@ static int default_lockmgr_cb(void **arg, enum
> AVLockOp op)
>      case AV_LOCK_DESTROY:
>          if (*mutex)
>              pthread_mutex_destroy(*mutex);
> -        av_free(*mutex);
> -        avpriv_atomic_ptr_cas(mutex, *mutex, NULL);
> +        av_freep(*mutex);
>

av_freep(mutex)? Probably why Michael is seeing the crash.

Timothy
diff mbox

Patch

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index de2dcea54d..17388ef637 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -26,7 +26,6 @@ 
  */
 
 #include "config.h"
-#include "libavutil/atomic.h"
 #include "libavutil/attributes.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
@@ -85,9 +84,11 @@  static int default_lockmgr_cb(void **arg, enum AVLockOp op)
                 av_free(tmp);
                 return AVERROR(err);
             }
-            if (avpriv_atomic_ptr_cas(mutex, NULL, tmp)) {
+            if (*mutex) {
                 pthread_mutex_destroy(tmp);
                 av_free(tmp);
+            } else {
+                *mutex = tmp;
             }
         }
 
@@ -103,8 +104,7 @@  static int default_lockmgr_cb(void **arg, enum AVLockOp op)
     case AV_LOCK_DESTROY:
         if (*mutex)
             pthread_mutex_destroy(*mutex);
-        av_free(*mutex);
-        avpriv_atomic_ptr_cas(mutex, *mutex, NULL);
+        av_freep(*mutex);
         return 0;
     }
     return 1;