[Scons-users] Possible 2.5.1 regression on Windows? (was: Unreliable build problem)

Hill, Steve (FP COM) Steve.Hill at cobham.com
Mon May 8 04:20:44 EDT 2017


The original patch is to work around the Python behaviour of creating inheritable file handles by default (which I believe has changed in the later versions of Python 3), which causes permission denied issues with multiple processes. The patch uses pywin32 to remove the inheritable flag after the file has been opened. There is obviously a race here but I did try putting a lock around this and the issue remained.

My hack reimplements (rather than augments) the file open functionality to open files as non-inheritable in the first place (and, as a side-effect, remove the dependency on pywin32). In theory this should have the same effect as the current patch in SCons but it seems to resolve the issue for me (and for my guinea-pig developer too). I'm aware that this is not a complete reimplementation (as yours is) but it is a small hack that provides more information about the problem...

S.

-- 
From: Scons-users [mailto:scons-users-bounces at scons.org] On Behalf Of Jason Kenny
Sent: 05 May 2017 17:47
To: SCons users mailing list
Subject: Re: [Scons-users] Possible 2.5.1 regression on Windows? (was: Unreliable build problem)

I am unclear what the original patch was trying to solve. It break the ability to do some redirection of the cons: device on windows. The addition of the patch I pushed was to set a Shared Write bit on win32. The mscrt fopen() and iostreams open() but do not allow passing of the shared write bit when opening files. The means that python and other programs not using the win32 File API directly can break. The addition allow me to fix two issues on windows that need this bit set.

1) hardlinks - the way windows handles this requires all applications to have Shared write. Without it an app pointing to a given hardlink cannot delete or write to the file ( even if the path is different for the application without shared write is using a different path)
2) deal with the issue of AV and search tools scanning the disk. For these tools to work that have to use shared read and write so they don't break the system application running. Classic C mscrt programs don't have shared write ( ie python) so they get upset with AV and search/indexing tools on the system. 

I had reported the issue a while back (5 years ago I think) to MS and they were going to add an ability to pass to fopen() the shared write flags. They were concerned that adding it by default might break some program that expecting current behavior. I assume the below patch is getting this value in with the new versions of the libraries we are using.

My patch also address issues in which scons will not get stat data correctly on symlinks on windows ( as it views it as a file not as symlink)

Jason

-----Original Message-----
From: Scons-users [mailto:scons-users-bounces at scons.org] On Behalf Of Hill, Steve (FP COM)
Sent: Friday, May 5, 2017 7:47 AM
To: SCons users mailing list <scons-users at scons.org>
Subject: Re: [Scons-users] Possible 2.5.1 regression on Windows? (was: Unreliable build problem)

That's interesting. Since you found the need to do that, I decided to try a much simpler patch^H^H^H^H^H hack to see whether it had an effect:

    def another_open(name, mode="r", *args, **kwds):
        flags = os.O_NOINHERIT

        read = "r" in mode
        write = "w" in mode
        append = "a" in mode
        rw = "+" in mode
        binary = "b" in mode

        if not read and not write and not append:
            read = True

        flags |= os.O_BINARY if binary else os.O_TEXT

        if not read:
            flags |= os.O_CREAT

        if write:
            flags |= os.O_TRUNC
        elif append:
            flags |= os.O_APPEND

        flags |= os.O_RDWR if rw else os.O_RDONLY if read else os.O_WRONLY

        fd = os.open(name, flags)
        return os.fdopen(fd, mode, *args, **kwds)

    import __builtin__
    __builtin__.open = another_open
    __builtin__.file = another_open

I've performed a number of builds (each producing around 700 executables) and it hasn't failed once. When I removed the patch, the first build failed with permission denied.

I've no idea why this has made a difference, since I assume that it is basically doing the same thing as _scons_open and scons_file (once the lock was put in place). I'll try to get one of the guinea-pigs that are trying 2.5.1 to put this in place and see if it resolves the issue for them too...

S.

--
From: Scons-users [mailto:scons-users-bounces at scons.org] On Behalf Of Jason Kenny
Sent: 04 May 2017 21:30
To: SCons users mailing list
Subject: Re: [Scons-users] Possible 2.5.1 regression on Windows? (was: Unreliable build problem)

I am coming in late on this.. but the code in question always broke my windows builds

Plus is break tools the MS tools that use hardlink or symlinks behind the covers to speed up work.


The way I got around this was this file I have in Parts. It a bigger monkey patch to the whole file code in python to allow it open files with the correct base permissions. It bypasses the c like fopen() problems. Since Parts uses a function that calls subprocess for the SPAWN logic this should work as well.

Anyways this is the file in Parts. It is setup to be a general easy drop in file to SCons

https://bitbucket.org/sconsparts/parts/src/bbe2af951c83e976782930bb8ec9f5466560286b/parts/overrides/os_file.py?at=master&fileviewer=file-view-default 

Jason





From: Scons-users [mailto:scons-users-bounces at scons.org] On Behalf Of Bill Deegan
Sent: Thursday, May 4, 2017 10:15 AM
To: SCons users mailing list <scons-users at scons.org>
Subject: Re: [Scons-users] Possible 2.5.1 regression on Windows? (was: Unreliable build problem)

Steve,
Can you try something since you've got a pretty reproducible configuration.
Add this to your top level SConstruct

sys.setcheckinterval(1000)
(This changes the thread swap checking interval, default on py2.7 is 100) Thanks, Bill


On Thu, May 4, 2017 at 12:02 AM, William Blevins <wblevins001 at gmail.com> wrote:
Fair enough. I was just remembering the hot topic of race conditions on Windows and trying to drudge up some recent information ;)

On Wed, May 3, 2017 at 9:53 PM, Bill Deegan <bill at baddogconsulting.com> wrote:
> Nope.. actually it's all 3.
>
> The issue (at it's root) is likely either threads are sharing the 
> handle and it's not closed in a timely fashion on windows, or just 
> plain normal file closes aren't being closed before the call returns on win32.
> Of note, looking at the c code, the GIL is released around the fclose()..
>
> -Bill
>
> On Wed, May 3, 2017 at 9:09 PM, William Blevins 
> <wblevins001 at gmail.com>
> wrote:
>>
>> Bill,
>>
>> I think you mean:
>> http://scons.tigris.org/issues/show_bug.cgi?id=2449
>>
>> https://bitbucket.org/scons/scons/pull-requests/389/fix-race-conditio
>> n-on-win32/diff
>>
>> V/R,
>> William
>>
>> On Wed, May 3, 2017 at 6:32 PM, Bill Deegan 
>> <bill at baddogconsulting.com>
>> wrote:
>> > Likely it's the same issue as this:
>> > http://scons.tigris.org/issues/show_bug.cgi?id=2124
>> >
>> >
>> >
>> > On Wed, May 3, 2017 at 3:14 PM, Arvid Rosén <arvid at softube.com> wrote:
>> >>
>> >> Hi,
>> >>
>> >> I have problems with that too. Typically running 16 threads. I had 
>> >> to rewrite all actions that generated source files (like header 
>> >> files with version information), because they never worked reliably on Windows.
>> >> Mac has
>> >> always been fine. I ended up using static header files with 
>> >> defines passed on the command line instead. Somewhat ugly but it 
>> >> works.
>> >>
>> >> I always thought this was rather related to some Windows related 
>> >> scanning or anti-virus service, but I might give it a try with
>> >> 2.3.6 and see if that solves it.
>> >>
>> >> Cheers,
>> >> Arvid
>> >>
>> >> Get Outlook for iOS
>> >> _____________________________
>> >> From: Bill Deegan <bill at baddogconsulting.com>
>> >> Sent: onsdag, maj 3, 2017 8:25 em
>> >> Subject: Re: [Scons-users] Possible 2.5.1 regression on Windows? (was:
>> >> Unreliable build problem)
>> >> To: SCons users mailing list <scons-users at scons.org>
>> >>
>> >>
>> >>
>> >> Steve,
>> >>
>> >> That's useful to know 2.3.6 isn't showing this.
>> >> We've had a few reports of others running into it.
>> >>
>> >> Are you using CacheDirs?
>> >>
>> >> -Bill
>> >>
>> >> On Wed, May 3, 2017 at 10:42 AM, Hill, Steve (FP COM) 
>> >> <Steve.Hill at cobham.com> wrote:
>> >>>
>> >>> Hi all,
>> >>>
>> >>> While looking at the unreliable build problem (which I will 
>> >>> return to), I upgraded to 2.5.1 (from 2.3.6). Unfortunately, this 
>> >>> seems to have introduced an issue which is preventing me from 
>> >>> rolling the new version out the development community: I am 
>> >>> seeing the file handle inheritance problem again that _scons_file 
>> >>> and _scons_open were added to work around.
>> >>>
>> >>> We typically use between 8 and 12 build threads (depending on the 
>> >>> physical machine that we are building on) and, when building with 
>> >>> 2.5.1, we quite frequently see the build fail with some sort of 
>> >>> access denied issue.
>> >>> The build system automatically runs handle.exe in this case and I 
>> >>> can see that the other (unrelated) build threads have an open 
>> >>> handle on the file at this time. Reverting to 2.3.6 results in 
>> >>> the issue going away.
>> >>>
>> >>> I've confirmed that _scons_open and _scons_file are both in place 
>> >>> for the built-in file() and open() and I've monkey patched 
>> >>> os.open to assert that it is always called with the 
>> >>> os.O_NOINHERIT flag. Does anyone know what other functions could 
>> >>> be causing this that I can check?
>> >>>
>> >>> Thanks,
>> >>>
>> >>> S.
>> >>>
>> >>> _______________________________________________
>> >>> Scons-users mailing list
>> >>> Scons-users at scons.org
>> >>> https://pairlist4.pair.net/mailman/listinfo/scons-users
>> >>>
>> >>
>> >>
>> >>
>> >>
>> >> _______________________________________________
>> >> Scons-users mailing list
>> >> Scons-users at scons.org
>> >> https://pairlist4.pair.net/mailman/listinfo/scons-users
>> >>
>> >
>> >
>> > _______________________________________________
>> > Scons-users mailing list
>> > Scons-users at scons.org
>> > https://pairlist4.pair.net/mailman/listinfo/scons-users
>> >
>> _______________________________________________
>> Scons-users mailing list
>> Scons-users at scons.org
>> https://pairlist4.pair.net/mailman/listinfo/scons-users
>
>
>
> _______________________________________________
> Scons-users mailing list
> Scons-users at scons.org
> https://pairlist4.pair.net/mailman/listinfo/scons-users
>
_______________________________________________
Scons-users mailing list
Scons-users at scons.org
https://pairlist4.pair.net/mailman/listinfo/scons-users

_______________________________________________
Scons-users mailing list
Scons-users at scons.org
https://pairlist4.pair.net/mailman/listinfo/scons-users
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 526 bytes
Desc: not available
URL: <https://pairlist4.pair.net/pipermail/scons-users/attachments/20170508/512a4524/attachment.pgp>


More information about the Scons-users mailing list