Обсуждение: Isn't HANDLE 64 bits on Win64?
... and if so, isn't postmaster.c's code to transfer a HANDLE value to a
child process all wet?
sprintf(paramHandleStr, "%lu", (DWORD) paramHandle);
...
paramHandle = (HANDLE) atol(id);
BTW, it seems like it'd be a good thing if we had a Win64 machine in the
buildfarm.
regards, tom lane
On Tue, Nov 16, 2010 at 01:35, Tom Lane <tgl@sss.pgh.pa.us> wrote: > ... and if so, isn't postmaster.c's code to transfer a HANDLE value to a > child process all wet? It is definitely 64-bit. sizeof(HANDLE)==8. So yes, it looks completely broken. I guess Windows doesn't actually *assign* you a handle larger than 2^32 until you actually ahve that many open handles. Typical values on my test system (win64) comes out at around 4000 in all tests. > BTW, it seems like it'd be a good thing if we had a Win64 machine in the > buildfarm. Yes. I actually thought we had one. Dave, weren't you going to set one up? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
On Tue, Nov 16, 2010 at 10:01 AM, Magnus Hagander <magnus@hagander.net> wrote: > On Tue, Nov 16, 2010 at 01:35, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> ... and if so, isn't postmaster.c's code to transfer a HANDLE value to a >> child process all wet? > > It is definitely 64-bit. sizeof(HANDLE)==8. > > So yes, it looks completely broken. I guess Windows doesn't actually > *assign* you a handle larger than 2^32 until you actually ahve that > many open handles. Typical values on my test system (win64) comes out > at around 4000 in all tests. > > >> BTW, it seems like it'd be a good thing if we had a Win64 machine in the >> buildfarm. > > Yes. I actually thought we had one. Dave, weren't you going to set one up? I was, but I saw one there so didn't bother (hamerkop). Windows buildfarm critters can take a surprising amount of herding... -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Nov 16, 2010 at 11:01, Magnus Hagander <magnus@hagander.net> wrote: > On Tue, Nov 16, 2010 at 01:35, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> ... and if so, isn't postmaster.c's code to transfer a HANDLE value to a >> child process all wet? > > It is definitely 64-bit. sizeof(HANDLE)==8. > > So yes, it looks completely broken. I guess Windows doesn't actually > *assign* you a handle larger than 2^32 until you actually ahve that > many open handles. Typical values on my test system (win64) comes out > at around 4000 in all tests. Patch applied for this and backpatched to 9.0. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
> On Tue, Nov 16, 2010 at 11:01, Magnus Hagander <magnus@hagander.net> wrote:
>> So yes, it looks completely broken. I guess Windows doesn't actually
>> *assign* you a handle larger than 2^32 until you actually ahve that
>> many open handles. Typical values on my test system (win64) comes out
>> at around 4000 in all tests.
> Patch applied for this and backpatched to 9.0.
I did a bit of googling and found some references claiming that Win64
will never assign system handles that are outside the range
representable as a signed long; and further stating there are standard
macros HandleToLong and LongToHandle to perform those conversions.
So I'd be comfortable with the original coding as long as we used those
macros instead of random casting. Dunno if you think that'd be cleaner
than what you did. (It's also a fair question whether those macros
are available on Win32.)
regards, tom lane
Dave Page <dpage@pgadmin.org> writes:
> On Tue, Nov 16, 2010 at 10:01 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> On Tue, Nov 16, 2010 at 01:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> BTW, it seems like it'd be a good thing if we had a Win64 machine in the
>>> buildfarm.
>> Yes. I actually thought we had one. Dave, weren't you going to set one up?
> I was, but I saw one there so didn't bother (hamerkop). Windows
> buildfarm critters can take a surprising amount of herding...
hamerkop seems to have gone AWOL around the time of the git conversion.
regards, tom lane
On Tue, Nov 16, 2010 at 15:42, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Tue, Nov 16, 2010 at 11:01, Magnus Hagander <magnus@hagander.net> wrote: >>> So yes, it looks completely broken. I guess Windows doesn't actually >>> *assign* you a handle larger than 2^32 until you actually ahve that >>> many open handles. Typical values on my test system (win64) comes out >>> at around 4000 in all tests. > >> Patch applied for this and backpatched to 9.0. > > I did a bit of googling and found some references claiming that Win64 > will never assign system handles that are outside the range > representable as a signed long; and further stating there are standard > macros HandleToLong and LongToHandle to perform those conversions. > So I'd be comfortable with the original coding as long as we used those > macros instead of random casting. Dunno if you think that'd be cleaner > than what you did. (It's also a fair question whether those macros > are available on Win32.) The one I found was: http://msdn.microsoft.com/en-us/library/aa384242(VS.85).aspx Which only talks about pointers, though - but handles are pointers, in theory. Do you still have a reference to the page that said they will never be assigned that high? I can't even find the proper documentation for those macros. I in the headers it looks like they'll be on Win32 - whether they're in *mingw* is a different issue altogether of course. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
On Tue, Nov 16, 2010 at 16:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Do you still have a reference to the page that said they will never be >> assigned that high? > > http://msdn.microsoft.com/en-us/library/ms810720.aspx > > which says > > USER and GDI handles are sign extended 32b values > > To facilitate the porting, a decision has been made that these system > handles should stay as 32b values, sign extended to 64b on the 64b > platform. That is, the individual handle types are still based on the > HANDLE type, which maps to void *, and so the size of the handle is the > size of the pointer, i.e. 4 bytes on 32b and 8 bytes on 64b. However, > the actual value of the handle on the 64b platform, (i.e. the meaningful > bits), fits within the lower 32b, while the upper bits just carry the > sign. > > This should make it easy to port the majority of the application > code. Handling of the special values, like -1, should be fairly > transparent. It also should agree nicely with all the cases where the > handles had been remoted with the help of the IDL definitions from the > public file wtypes.idl. However, care needs to be taken when remoting > the handles was done via a DWORD, as the upper long should be properly > sign extended on the 64b side. The app should use HandleToLong() and > LongToHandle() macros (inline functions) to do the casting right. > > What's not clear to me is whether the section title means that only > certain handles have this guarantee, and if so whether we have to worry > about running into ones that don't. I think it is pretty clear it does - the section has a list of different handles at the bottom. What we're using is a File Mapping Object, which is not on that list. And which is, AFAICT, not a user or gdi handle. That doesn't mean it's not guaranteed to be in the 32-bit space, but I'm pretty sure that specific page doesn't guarantee it. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > On Tue, Nov 16, 2010 at 16:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> What's not clear to me is whether the section title means that only >> certain handles have this guarantee, and if so whether we have to worry >> about running into ones that don't. > I think it is pretty clear it does - the section has a list of > different handles at the bottom. What we're using is a File Mapping > Object, which is not on that list. And which is, AFAICT, not a user or > gdi handle. > That doesn't mean it's not guaranteed to be in the 32-bit space, but > I'm pretty sure that specific page doesn't guarantee it. Well, the patch as-applied is fine with me. I just wanted to be sure we'd considered the alternatives, especially in view of the fact that we have not seen any clear failures of the previous coding. The reason this came to mind was http://archives.postgresql.org/pgsql-admin/2010-11/msg00128.php which looks for all the world like a handle transmission failure --- but that person claims to be running Win32, so unless he's wrong, this particular issue doesn't explain his problem. regards, tom lane
On Tue, Nov 16, 2010 at 16:35, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Tue, Nov 16, 2010 at 16:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> What's not clear to me is whether the section title means that only >>> certain handles have this guarantee, and if so whether we have to worry >>> about running into ones that don't. > >> I think it is pretty clear it does - the section has a list of >> different handles at the bottom. What we're using is a File Mapping >> Object, which is not on that list. And which is, AFAICT, not a user or >> gdi handle. > >> That doesn't mean it's not guaranteed to be in the 32-bit space, but >> I'm pretty sure that specific page doesn't guarantee it. > > Well, the patch as-applied is fine with me. I just wanted to be sure > we'd considered the alternatives, especially in view of the fact that > we have not seen any clear failures of the previous coding. Check. > The reason this came to mind was > http://archives.postgresql.org/pgsql-admin/2010-11/msg00128.php > which looks for all the world like a handle transmission failure > --- but that person claims to be running Win32, so unless he's > wrong, this particular issue doesn't explain his problem. Yeah. Error 6 is the infamous "the handle is invalid", which says very very little about what's actually the problem. It could be that it gets the wrong handle over the wire, but it seems fairly unlikely. Since we're talking file handles, I'm willing to start by going down the usual road of suspecting antivirus/antispyware if it's there... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > Do you still have a reference to the page that said they will never be > assigned that high? http://msdn.microsoft.com/en-us/library/ms810720.aspx which says USER and GDI handles are sign extended 32b values To facilitate the porting, a decision has been made that thesesystem handles should stay as 32b values, sign extended to 64b on the 64b platform. That is, the individual handletypes are still based on the HANDLE type, which maps to void *, and so the size of the handle is the size of thepointer, i.e. 4 bytes on 32b and 8 bytes on 64b. However, the actual value of the handle on the 64b platform, (i.e.the meaningful bits), fits within the lower 32b, while the upper bits just carry the sign. This should makeit easy to port the majority of the application code. Handling of the special values, like -1, should be fairly transparent.It also should agree nicely with all the cases where the handles had been remoted with the help of the IDLdefinitions from the public file wtypes.idl. However, care needs to be taken when remoting the handles was done viaa DWORD, as the upper long should be properly sign extended on the 64b side. The app should use HandleToLong() and LongToHandle() macros (inline functions) to do the casting right. What's not clear to me is whether the section title means that only certain handles have this guarantee, and if so whether we have to worry about running into ones that don't. regards, tom lane