Re: [1/5] qmgr: Add infrastructure for background file transferring. [take 2]

[Alexandre Julliard]

Dan Hipschman <dsh(a)linux.ucla.edu> writes:
> +DWORD WINAPI fileTransfer(void *param)
> +{
> +    BackgroundCopyManagerImpl *qmgr = &globalMgr;
> +    BackgroundCopyJobImpl *job = NULL;
> +
> +    for (;;)
> +    {
> +        /* Note that other threads may add files to the job list, but only
> +           this thread ever deletes them so we don't need to worry about jobs
> +           magically disappearing from the list.  */
> +        EnterCriticalSection(&qmgr->cs);
> +        {
> +            struct list *next = (job
> +                                 ? list_next(&qmgr->jobs, &job->entryFromQmgr)
> +                                 : list_head(&qmgr->jobs));
> +            job = (next
> +                   ? LIST_ENTRY(next, BackgroundCopyJobImpl, entryFromQmgr)
> +                   : NULL);
> +        }
> +        LeaveCriticalSection(&qmgr->cs);
> +
> +        if (job)
> +        {
> +            /* It's fine to access the job state outside of the job's critical
> +               section in these cases because this thread is the only one that
> +               can change the state once it gets in one of these.  */
> +            if (job->state == BG_JOB_STATE_ACKNOWLEDGED || job->state == BG_JOB_STATE_CANCELLED)
> +            {
> +                EnterCriticalSection(&qmgr->cs);
> +                list_remove(&job->entryFromQmgr);
> +                job->lpVtbl->Release((IBackgroundCopyJob *) job);
> +                LeaveCriticalSection(&qmgr->cs);
> +                job = NULL;
> +            }
> +            else
> +            {
> +                /* Process job */
> +            }
> +        }
> +        else
> +            Sleep(1000);

You should use some sort of synchronization mechanism, not just poll the
list every second.

-- 
Alexandre Julliard
julliard(a)winehq.org