Re: [boost] [afio] AFIO review postponed till Monday

Subject: Re: [boost] [afio] AFIO review postponed till Monday
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-07-22 21:11:14

On 22 Jul 2015 at 13:35, Glen Fernandes wrote:

> 1. Use of documented kernel APIs: I'm torn here; on one hand, I don't
> condemn use of them and I would agree with Asbjørn that it isn't a show
> stopper. Yet you have a case of an actual show-stopper (crash) in AFIO
> because of the use of them.[1] 

The show stopper crash is due to how ASIO implements its IOCP reactor 
which just happens to collide with the WOW64 bug. If I could modify 
the ASIO source code to special case this it's fixed. This is why 
Microsoft marked it as wonfix because even though it's a bug in their 
code, it is really super easy to work around if you modify your IOCP 
reactor.

As I mentioned before, if I don't hand off the async to ASIO I make 
the problem go away.

> 2. Use of undocumented or deprecated APIs: Kernel APIs or otherwise, even if
> apparently innocuous, this practice is alarming to me. I would be surprised
> if it doesn't taint the review of AFIO.

I can replace ObjectNameInformation with a new routine which uses 
non-deprecated APIs if the review demands it. Indeed, the 
ObjectNameInformation use replaced some original code which iterated 
every mounted volume in the system per path lookup, which was costly.

I suppose one _could_ make the assumption that open file handles can 
never be relocated across mounted volumes and that mounted volumes 
are never remounted anywhere new while file handles are open within 
them. With those two assumptions I can use NtQueryFileInformation 
instead, and reconstruct the same thing NtQueryObject returns.

What worried me was the possibility that a volume could be mounted in 
two or more locations you see, and that the canonical path of a file 
could change, hence the iteration of all mounted volumes to check for 
that.

> > I think this is the wrong question to ask.  The right question to ask is:
> > [snip]
> 
> So the wrong question to ask is, "Is it concerning that AFIO uses these APIs
> given your use of them is unsupported?" and the right question to ask is "Is
> it concerning that AFIO use these APIs given your use of them is unsupported
> but the advantages are [...]?"  ?  :-) 

There is unsupported and there is unsupported. If you want to write a 
user mode program which executes in the Windows boot sequence before 
the subsystems start up (i.e. there is no win32 API available yet), 
then you write that program using the NT kernel API. That's 
supported, because it's categorised as a DDK user mode application.

By unsupported Microsoft have never said "don't use these APIs 
directly", they actually said "we won't promise we won't change 
anything inside the header file Winternl.h" and it just happens that 
the kernel API falls into that header file along with a ton of other 
stuff. I may have mentioned earlier that Cygwin and Mingw use the 
same APIs as AFIO, and if you review the Windows bug tracker where 
those projects report bugs in Windows you'll find a common pattern of 
the first tier support in Microsoft refusing the fix the bug as its 
use is "unsupported", then a more senior tier fixes the bug anyway.

That's because the single biggest user of the NT kernel API is 
Microsoft itself, and Microsoft would internally like bugs in one of 
its most fundamental APIs fixed, so they take these bug reports 
seriously.

If this isn't "supported", it looks awfully like supported. I would 
actually say the real situation is the Microsoft doesn't _encourage_ 
direct user mode use of NT kernel APIs. Historically speaking it 
wouldn't have worked on WinCE and Win95, and that was once a problem. 
I think we're a long way away from that now.

BTW the Visual Studio MSVCRT makes extensive use of NT kernel calls 
itself, indeed Stephan pointed me at RtlGenRandom which solved a 
problem for me.

> Niall Douglas wrote:
> > I would say all three of these yes. 
> 
> Sounds like a good enough spot to put yourself out of your misery and take
> this opportunity to escape the review queue that you've been in for so long.
> I understand wanting to improve the library, but something tells me you
> don't want to be in the queue for another two years constantly refactoring
> AFIO. Just my two cents.

I would certainly prefer review sooner rather than later. It could be 
if AFIO had been reviewed much earlier on I wouldn't need to do so 
much refactoring now, but maybe it's too niche for that. I don't 
know, I haven't even proved the case for futures instead of 
async_result yet, and I already know the ASIO crowd here is going to 
be united on that.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ 
http://ie.linkedin.com/in/nialldouglas/