#440 mingw-get: Avoid hanging at DLL initialization

Patch_committed
closed-accepted
mingw-get (5)
2010-03-30
2010-03-28
No

When I run mingw-get, it hangs for me at initialization, every single time. I tried both the pre-compiled version available at the download page, as well as building my own. For the record, I have Windows XP Home SP3.

Using gdb, I found the hang occurs at src/pkginet.cpp (pkgInternetAgent::pkgInternetAgent), when calling InternetAttemptConnect. The backtrace indicated that this constructor was called during DLL initialization (DLLMain), before even running the main() function. This seems to be due to pkgDownloadAgent being a static object of this class.

According to the MSDN page for DLLMain:
http://msdn.microsoft.com/en-us/library/ms682583%28VS.85%29.aspx
"The entry-point function should perform only simple initialization or termination tasks".
"Calling functions that require DLLs other than Kernel32.dll may result in problems that are difficult to diagnose"
"Because DLL notifications are serialized, entry-point functions should not attempt to communicate with other threads or processes. Deadlocks may occur as a result"

Anyway, moving the InternetAttemptConnect outside the constructor solved the issue for me.

Discussion

  • Cesar Strauss

    Cesar Strauss - 2010-03-28
    • summary: migw-get:Avoid hanging at DLL initialization --> mingw-get: Avoid hanging at DLL initialization
     
  • Keith Marshall

    Keith Marshall - 2010-03-29

    Hi Cesar,

    Thanks for debugging this. Just one minor issue: in the code you moved out of the constructor, you now gratuitously check the value of `SessionHandle', without any prior initialisation for it. Surely, it would be safer to have the constructor initialise it to NULL?

    Maybe we could also consider:

    if( (SessionHandle == NULL)
    && (InternetAttempConnect( 0 ) == ERROR_SUCCESS) )
    SessionHandle = ...

    in preference to:

    if( !SessionHandle )
    if( InternetAttemptConnect( 0 ) == ERROR_SUCCESS )
    SessionHandle = ...

     
  • Cesar Strauss

    Cesar Strauss - 2010-03-30

    > you now gratuitously check the value of
    > `SessionHandle', without any prior initialisation for it.
    > Surely, it would be safer to have the constructor
    > initialise it to NULL?

    Good catch. However, I was about to add it to the constructor initialization list, when I saw it was already there!

    ...
    public:
    inline pkgInternetAgent():SessionHandle( NULL )
    ...

    I guess defensive programming really pays off.

    I attach a new patch which improves the code style according to your suggestion.

     
  • Keith Marshall

    Keith Marshall - 2010-03-30
    • labels: --> mingw-get
    • milestone: --> Patch_committed
    • assigned_to: nobody --> keithmarshall
    • status: open --> closed-accepted
     
  • Keith Marshall

    Keith Marshall - 2010-03-30

    Applied thanks, (with some comment embellishments).

    Just something to watch, maybe: after I applied this, and tested in Wine, I had an apparent process hang on first attempt to download msysCORE. I've never noticed this before, but after aborting that one attempt, (with SIGINT), I've been unable to reproduce the effect.