[ros-dev] confusing smss routine (ea)

[ea]

Hi Gunnar,

> I was doing some changes to RtlCreateUserProcess and saw the strage 
> rotuine SmCreateUserProcess:
>
> http://svn.reactos.com/viewcvs/trunk/reactos/subsys/smss/smapiexec.c?rev=14244&view=markup 
>
>
> It has several bugs/problems/confusions:

Sorry, SM is not finished yet.

> 1) RtlDestroyProcessParameters is not called if RtlCreateUserProcess 
> failed.

This is a bug.

> 2) If WaitForIt is TRUE and you specify a timeout, the caller in not 
> notified if SmCreateUserProcess returned due to timeout. The process 
> will stay running forever.

This is a bug.

> 3) If TerminateIt is TRUE, handles are closed. This is 
> wrong/confusing. Closing the handles does not terminate the process 
> (wrong name?).

This is a bug.

> 4) If you dont pass a UserProcessInfo and TerminateIt is FALSE,
> it will leak thread/process handles.

This may not be a bug, but, as you noted, a badly chosen name. The 
intended use of the TerminateIt is for native processes, which handles 
SM is not interested in, when (if) they terminate. A native process, 
which is not a subsystem server, could run forever (see the native shell 
Steven proposed a few days ago).

> 5) If you pass UserProcessInfo and TerminateIt is TRUE, the 
> thread/process handles in UserProcessInfo will be invalid.

This is an intended behaviour: if TerminateIt==TRUE I am not interested 
in those handles. SM is interested only in subsystems, which it will 
manage (as it self-register for managing such images). Maybe running 
native boot-time processes should be done after self-registering, but it 
would be useless, because those processes do not need to connect to 
\SmApiPort.

>
> etc.etc.
>
> VERY confusing and bug prone:
>
I am sorry: I have got neither the sources, nor the documents David 
Cutler wrote.

> SmCreateUserProcess is used once in the end of this file and it pass 
> FALSE for TerminateIt and pass no UserProcessInfo thus the 
> thread/process handles will never be closed.

It is used also in initrun.c, for running native boot-time processes 
(autocheck, chkreg et sim.) before any environment subsystem is active.

>
>
> I should have fixed it myself if i just understod how this routine is 
> _supposed_ to work:-D

This is a critical routine, because every child process SM creates is 
originated there. But current bugs, I am sure it will need more work to 
match that role.

Please feel free to fix any bug you find: I am not the owner of the SM! 
:) I just tried to implement how I gussed things work there (do not 
exclude the case I am wrong, I am a bad programmer etc.).

I am sorry I can't fix the bugs right now, but I'll be busy for a few 
more days.

Emanuele Aliberti