Re: [Qemu-devel] [PATCH] ui/vnc-jobs: Delete unused and buggy vnc_stop_worker_thread()

[Peter Maydell]

Another bug:

(4) if vnc_job_push() discovers that queue->exit is true it
will free the job but leak its rectangle list
(5) the early-exit ("goto disconnected") code paths in
vnc_worker_thread_loop() also leak the rectangle list

And a couple of inefficiencies/oddities which aren't actually bugs:

(6) An inefficiency (unnecessary lock/unlocks):
the code locks and unlocks the queue in vnc_job_new() and
vnc_job_add_rect() when it is manipulating the job->rectangles list.
In the former case this is definitely pointless -- we've just alloc'd
the VncJob struct so it's impossible for anybody else to be trying to
use the list yet. In the latter it's not necessary since the semantics
are that we create a job with vnc_job_new, fill it up with rectangles
and then hand it off to the queue with vnc_job_push(). So we know
only one thread will be touching the job before it goes in the queue
and grabbing the queue lock is unnecessary overhead.

(7) vnc_has_job() is unused, which is just as well because it's
pretty hard to use non-racily, since the true/false answer it
returns could be made wrong as soon as it drops the queue lock.
vnc_has_job_locked() is OK (as is its usage pattern in _join).

-- PMM