vidmix: coverity fix

[alfredh]

New defect(s) Reported-by: Coverity Scan
Showing 12 of 12 defect(s)

** CID 462177: Concurrent data access violations (MISSING_LOCK)
/rem/vidmix/vidmix.c: 71 in clear_all()

________________________________________________________________________________________________________ *** CID 462177: Concurrent data access violations (MISSING_LOCK) /rem/vidmix/vidmix.c: 71 in clear_all()
65 struct le *le;
66
67 for (le=mix->srcl.head; le; le=le->next) {
68
69 struct vidmix_source *src = le->data;
70

CID 462177:  Concurrent data access violations  (MISSING_LOCK)
Accessing "src->clear" without holding lock "vidmix_source.mutex". Elsewhere, "vidmix_source.clear" is accessed with "vidmix_source.mutex" held 5 out of 6 times.

71 src->clear = true;
72 }
73 }
74
75
76 static void destructor(void *arg)

** CID 462176: Concurrent data access violations (MISSING_LOCK)
/rem/aubuf/ajb.c: 155 in ajb_alloc()

[sreimers]

Looks like our coverity CI setup is broken, has no reports about this.

[alfredh]

there is only one warning from coverity on baresip/re:

 1/* */
 2#include <sys/types.h>
  	
CID 301203 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)
1. cannot_open_file: cannot open source file "sys/event.h"
 3#include <sys/event.h>
 4
 5int main(int argc, char** argv)
 6{
 7  (void)argv;
 8#ifndef kqueue
 9  return ((int*)(&kqueue))[argc];
10#else
11  (void)argc;
12  return 0;
13#endif
14}

[sreimers]

It looks like this is from the cmake -B build part, how did you compile and upload?
Maybe we need only to split this. Will try something.

[alfredh]

I have used a manual build and upload:

$ cmake .
$ ~/tmp/cov-analysis-linux64-2022.12.2/bin/cov-build --dir cov-int make
$ tar czvf re.tgz cov-int

[sreimers]

Thanks, CI setup works now (is triggered currently only by tags or coverity branch, since coverity limits daily uploads) and shows all bugs. Now we can fix them :-)

[sreimers]

Looks like this new locking produces a potential deadlock:

WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=186275)
  Cycle in lock order graph: M0 (0x7b4400004e40) => M1 (0x7b3400000bf0) => M0

  Mutex M1 acquired here while holding mutex M0 in thread T7:
    #0 pthread_mutex_lock <null> (slmix+0x27a381) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #1 mtx_lock /re/src/thread/posix.c:180:10 (slmix+0x5ae13a) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #2 vidmix_thread /re/rem/vidmix/vidmix.c:220:3 (slmix+0x5efded) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #3 handler /re/src/thread/thread.c:101:9 (slmix+0x4fe098) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #4 handler /re/src/thread/posix.c:23:27 (slmix+0x5ad8fc) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)

    Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message

  Mutex M0 acquired here while holding mutex M1 in main thread:
    #0 pthread_mutex_lock <null> (slmix+0x27a381) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #1 mtx_lock /re/src/thread/posix.c:180:10 (slmix+0x5ae13a) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #2 clear_all /re/rem/vidmix/vidmix.c:69:3 (slmix+0x5eeca8) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #3 vidmix_source_enable /re/rem/vidmix/vidmix.c:512:2 (slmix+0x5ee7b6) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #4 disp_enable_h /modules/vmix/vmix.c:119:2 (slmix+0x3e259b) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #5 slmix_disp_enable /src/mix.c:384:9 (slmix+0x2d3a98) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #6 slmix_session_video /src/sess.c:458:3 (slmix+0x2d7c28) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #7 http_req_handler /src/http.c:372:3 (slmix+0x2d0435) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #8 recv_handler /re/src/http/server.c:281:4 (slmix+0x45a1c6) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #9 tcp_recv_handler /re/src/tcp/tcp.c:442:3 (slmix+0x4f394e) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #10 fd_poll /re/src/main/main.c:802:4 (slmix+0x483439) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #11 re_main /re/src/main/main.c:976:9 (slmix+0x4802ba) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)
    #12 main /src/main.c:193:2 (slmix+0x2cca00) (BuildId: f7fd61d965732ab811ec233f56f3cf075d6b0245)

Should we revert this commit?

[sreimers]

We can replace src->clear with a atomic.

[sreimers]

Or we drop the optional clear, I had problems with artifacts in the past and simply clear always the background:

https://github.com/Studio-Link/mix/blob/main/patches/re_vidmix_clear.patch