NPFS problems

[Ge van Geldorp]

After changes r18883/r18912 umpnpmgr.exe (and eventlog.exe) don't start
anymore. The problem is that they try to connect to the service control
manager using a named pipe (advapi32/service/sctrl.c function
ScConnectControlPipe). Before opening the pipe, a call is made to
WaitNamedPipe() to see if the pipe exists. This call currently fails,
resulting in failure of ScConnectControlPipe(), resulting in failure of
umpnpmgr.exe, resulting in network drivers not being installed, resulting in
no network connectivity.

WaitNamedPipe() NtOpenFile()s the pipe and then issues a FSCTL_PIPE_WAIT
ioctl. The problem is that this end of the pipe is now connected to the
server end (which is already listening) during the call to NtOpenFile() (in
function NpfsCreate(), setting the PipeStatus to FILE_PIPE_CONNECTED_STATE.
NpfsWaitPipe first checks the PipeStatus and bails out if it's not 0
(passive waiting state). So, in our case NpfsWaitPipe fails, 'cause the pipe
is already connected. The patch below just makes NpfsWaitPipe return success
in case it's already connected, which solves the umpnpmgr problem. However,
I don't think it's the correct solution, after the FSCTL_PIPE_WAIT ioctl
returns WaitNamedPipe() will close its end, so the server end sees an
unexpected close.

I have no clue how to proceed, and to be honest I'm not interested in
solving it myself. I'd rather see the author of the original patch take his
responsibility and fix the regression he introduced.

GvG.

[Thomas Weidenmueller]

Ge van Geldorp wrote:
> After changes r18883/r18912 umpnpmgr.exe (and eventlog.exe) don't start
> anymore. The problem is that they try to connect to the service control
> manager using a named pipe (advapi32/service/sctrl.c function
> ScConnectControlPipe). Before opening the pipe, a call is made to
> WaitNamedPipe() to see if the pipe exists. This call currently fails,
> resulting in failure of ScConnectControlPipe(), resulting in failure of
> umpnpmgr.exe, resulting in network drivers not being installed, resulting in
> no network connectivity.
>
> WaitNamedPipe() NtOpenFile()s the pipe and then issues a FSCTL_PIPE_WAIT
> ioctl. The problem is that this end of the pipe is now connected to the
> server end (which is already listening) during the call to NtOpenFile() (in
> function NpfsCreate(), setting the PipeStatus to FILE_PIPE_CONNECTED_STATE.
> NpfsWaitPipe first checks the PipeStatus and bails out if it's not 0
> (passive waiting state). So, in our case NpfsWaitPipe fails, 'cause the pipe
> is already connected. The patch below just makes NpfsWaitPipe return success
> in case it's already connected, which solves the umpnpmgr problem. However,
> I don't think it's the correct solution, after the FSCTL_PIPE_WAIT ioctl
> returns WaitNamedPipe() will close its end, so the server end sees an
> unexpected close.
>
> I have no clue how to proceed, and to be honest I'm not interested in
> solving it myself. I'd rather see the author of the original patch take his
> responsibility and fix the regression he introduced.
>
> GvG.
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

Well, I can revert my kernel32 fixes, but then i guess that npfs doesn't
ever get fixed ;) If you want me to, i'll do it immediately of course.

- Thomas

[Casper Hornstrup]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Thomas
> Weidenmueller
> Sent: 4. november 2005 18:47
> To: ReactOS Development List
> Subject: Re: [ros-dev] NPFS problems
>
> Well, I can revert my kernel32 fixes, but then i guess that npfs doesn't
> ever get fixed ;) If you want me to, i'll do it immediately of course.
> Thomas

You don't think the changes should be reverted?

Casper

[WaxDragon]

I think someone should sit down with npfs and "kernel32_winetest pipe"
and sort out the numerous issues it reports.

WD

On 11/4/05, Casper Hornstrup [e-mail] wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On Behalf Of Thomas
> > Weidenmueller
> > Sent: 4. november 2005 18:47
> > To: ReactOS Development List
> > Subject: Re: [ros-dev] NPFS problems
> >
> > Well, I can revert my kernel32 fixes, but then i guess that npfs doesn't
> > ever get fixed ;) If you want me to, i'll do it immediately of course.
> > Thomas
>
> You don't think the changes should be reverted?
>
> Casper
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

--
<GreatLord> I did remove the dam fukect keyboard test

[Thomas Weidenmueller]

Casper Hornstrup wrote:
> You don't think the changes should be reverted?

As I said it's ok to revert them, but the changes are correct. Our npfs
is the problem and needs to be fixed. Unfortunately I'm not really
familiar with our npfs code, otherwise I would've taken a look at it and
fixed it in the first place. Herve is about to revert the changes for now ;)

- Thomas

[Hartmut Birr]

Thomas Weidenmueller wrote:
> Casper Hornstrup wrote:
> > You don't think the changes should be reverted?
>
> As I said it's ok to revert them, but the changes are correct. Our npfs
> is the problem and needs to be fixed. Unfortunately I'm not really
> familiar with our npfs code, otherwise I would've taken a look at it and
> fixed it in the first place. Herve is about to revert the changes for now ;)
> Thomas

Thomas's changes are correct. The problem is the special mode in npfs, 
which I've removed. The special mode was triggered by opening or 
creating a pipe with the share disposition read and write. This is 
wrong. I think, the special mode must be triggered, if the pipe is only 
open for read attributes.

- Hartmut

[Casper Hornstrup]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Hartmut Birr
> Sent: 4. november 2005 21:03
> To: ReactOS Development List
> Subject: Re: [ros-dev] NPFS problems
>
> Thomas Weidenmueller wrote:
> > Casper Hornstrup wrote:
> > > You don't think the changes should be reverted?
> >
> > As I said it's ok to revert them, but the changes are correct. Our npfs
> > is the problem and needs to be fixed. Unfortunately I'm not really
> > familiar with our npfs code, otherwise I would've taken a look at it and
> > fixed it in the first place. Herve is about to revert the changes for now ;)
> > Thomas
>
> Thomas's changes are correct. The problem is the special mode in npfs,
> which I've removed. The special mode was triggered by opening or
> creating a pipe with the share disposition read and write. This is
> wrong. I think, the special mode must be triggered, if the pipe is only
> open for read attributes.
> Hartmut

I'm sure they are, but I'm also sure that many would prefer "incorrect-and-working"
over "correct-and-non-working" if those were the only two choices.

Casper

[Alex Ionescu]

Casper Hornstrup wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On Behalf Of Hartmut Birr
> > Sent: 4. november 2005 21:03
> > To: ReactOS Development List
> > Subject: Re: [ros-dev] NPFS problems
> >
> > Thomas Weidenmueller wrote:
> > > Casper Hornstrup wrote:
> > > > You don't think the changes should be reverted?
> > >
> > > As I said it's ok to revert them, but the changes are correct. Our npfs
> > > is the problem and needs to be fixed. Unfortunately I'm not really
> > > familiar with our npfs code, otherwise I would've taken a look at it and
> > > fixed it in the first place. Herve is about to revert the changes for now ;)
> > > Thomas
> >
> > Thomas's changes are correct. The problem is the special mode in npfs,
> > which I've removed. The special mode was triggered by opening or
> > creating a pipe with the share disposition read and write. This is
> > wrong. I think, the special mode must be triggered, if the pipe is only
> > open for read attributes.
> > Hartmut
>
> I'm sure they are, but I'm also sure that many would prefer "incorrect-and-working"
> over "correct-and-non-working" if those were the only two choices.
>
> Casper

I wouldn't.

"incorrect-and-working" leads to stuff never being made correct.

Best regards,
Alex Ionescu

[TwoTailedFox]

Couldn't we make a branch with the applied Changes, and leave Trunk
with the non-applied Changes? At least that way, we'd have a decent
way of being able to fix NPFS, without the Trunk being affected by any
unwelcome changes

On 11/4/05, Alex Ionescu [e-mail] wrote:
> Casper Hornstrup wrote:
> > > -----Original Message-----
> > > From: [e-mail] [mailto:[e-mail]] On Behalf Of Hartmut Birr
> > > Sent: 4. november 2005 21:03
> > > To: ReactOS Development List
> > > Subject: Re: [ros-dev] NPFS problems
> > >
> > > Thomas Weidenmueller wrote:
> > > > Casper Hornstrup wrote:
> > > > > You don't think the changes should be reverted?
> > > >
> > > > As I said it's ok to revert them, but the changes are correct. Our npfs
> > > > is the problem and needs to be fixed. Unfortunately I'm not really
> > > > familiar with our npfs code, otherwise I would've taken a look at it and
> > > > fixed it in the first place. Herve is about to revert the changes for now ;)
> > > > Thomas
> > >
> > > Thomas's changes are correct. The problem is the special mode in npfs,
> > > which I've removed. The special mode was triggered by opening or
> > > creating a pipe with the share disposition read and write. This is
> > > wrong. I think, the special mode must be triggered, if the pipe is only
> > > open for read attributes.
> > > Hartmut
> >
> > I'm sure they are, but I'm also sure that many would prefer "incorrect-and-working"
> > over "correct-and-non-working" if those were the only two choices.
> >
> > Casper
>
> I wouldn't.
>
> "incorrect-and-working" leads to stuff never being made correct.
>
> Best regards,
> Alex Ionescu
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

--
"I had a handle on life, but then it broke"

[Casper Hornstrup]

Is the fix really that large?

Casper
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of TwoTailedFox
> Sent: 4. november 2005 23:30
> To: ReactOS Development List
> Subject: Re: [ros-dev] NPFS problems
>
> Couldn't we make a branch with the applied Changes, and leave Trunk
> with the non-applied Changes? At least that way, we'd have a decent
> way of being able to fix NPFS, without the Trunk being affected by any
> unwelcome changes

[Casper Hornstrup]

> I wouldn't.
>
> "incorrect-and-working" leads to stuff never being made correct.

How about making it "correct-and-working" in svn then?

Casper