Paul Gofman pgofman@codeweavers.com writes:
On 5/20/21 11:56, Alexandre Julliard wrote:
Paul Gofman pgofman@codeweavers.com writes:
On 5/20/21 10:56, Alexandre Julliard wrote:
Paul Gofman pgofman@codeweavers.com writes:
- job_handle_count = req->jobs_size / sizeof(*handles);
- for (i = 0; i < job_handle_count; ++i)
- {
if (!(job = get_job_obj( current->process, job_handles[i], JOB_OBJECT_ASSIGN_PROCESS ))){close( socket_fd );goto done;}release_object( job );- }
It doesn't seem useful to pre-check the handles, you'll need to handle failures when assigning the process anyway.
The reason I did it this way is that invalid handle aborts the process creation without changing any job's state, while error adding job to process leaves the state of the jobs earlier in the list altered if aborting on adding next job. So I probably need to precheck (get) the handles before assigning the process anyway. Should I move this check to that place (and squash the patches)?
I'd expect that the process cleanup would undo what was done to the previous jobs. Otherwise you can't allow add_job_process() to fail in patch 4.
But my tests in patch 4 show that this is not the case. When the process can't be assigned to the second job in list, the first one still gets the parent and even sends completions for adding and removing nonexistent process. My test, besides querying process counts from that unexpectedly altered job, checks that it got the parent by attempting another process create to be sure.
Ah, I see. Sounds very much like a Windows bug, but I guess we have to do the same then. Thanks!