Opened 8 years ago

Closed 7 years ago

#3605 closed bug (fixed)

Dll's freeze with -threaded

Reported by: NeilMitchell Owned by: simonmar
Priority: high Milestone: 7.0.1
Component: Documentation Version: 6.12.1 RC1
Keywords: Cc: ndmitchell@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Attached are two source files (one .c, one .hs), which are compiled (using mk.sh) into DsoHsDemo.dll. When this library is loaded, using the following C snippet:

int main(int argc, char* argv[])
{
    printf("Started LoadLibrary\n");
    LoadLibrary("DsoHsDemo.dll");
    printf("Finished LoadLibrary\n");
}

The program freezes:

$ ./mk.sh && ./CSnippet
The Glorious Glasgow Haskell Compilation System, version 6.12.0.20091010
Creating library file: DsoHsDemo.dll.a
Started LoadLibrary
pre hs_init
<program freezes>

The freeze only occurs when the dll is linked with -threaded, and happens within either startupHaskell, or (if you call hs_init/hs_add_root instead) within hs_add_root. This happens with GHC 6.10.4 and 6.12rc1 on Windows XP.

This bug appears to make it impossible to successfully build a DLL for multithreaded use.

Attachments (5)

HsDllMain.c (651 bytes) - added by NeilMitchell 8 years ago.
HsDemo.hs (64 bytes) - added by NeilMitchell 8 years ago.
HsDemo.2.hs (21 bytes) - added by NeilMitchell 8 years ago.
mk.sh (189 bytes) - added by NeilMitchell 8 years ago.
win32_dlls_documentation_bug_3605.patch (169.8 KB) - added by NeilMitchell 7 years ago.
Patch to the documentation

Download all attachments as: .zip

Change History (25)

Changed 8 years ago by NeilMitchell

Attachment: HsDllMain.c added

Changed 8 years ago by NeilMitchell

Attachment: HsDemo.hs added

Changed 8 years ago by NeilMitchell

Attachment: HsDemo.2.hs added

Changed 8 years ago by NeilMitchell

Attachment: mk.sh added

comment:1 Changed 8 years ago by NeilMitchell

Please ignore HsDemo.2.hs, I clicked on the wrong file. Either HsDemo file will cause the freeze.

comment:2 Changed 8 years ago by duncan

My initial reaction is that calling startupHaskell from within DllMain is a tad dodgy. The MSDN docs say there are pretty severe limitations on what can be done in it. In particular no calls to LoadLibrary (or other functions that call LoadLibrary). This is because the DllMain is called with a loader lock. So this is one possibility that is consistent with the observed freeze.

It also says:

Calling functions that require DLLs other than Kernel32.dll
may result in problems that are difficult to diagnose. For
example, calling User, Shell, and COM functions can cause
access violation errors, because some functions load other
system components.

So to determine if startupHaskell is safe would require a full audit of all the code it calls.

Another approach would be to trace the library load and see what OS / library calls it is making.

There are more reasons to suggest that startupHaskell cannot safely be run from within DllMain, the MSDN Best Practices for Creating DLLs document states that:

You should never perform the following tasks from within DllMain:

including:

Call CreateThread. Creating a thread can work if you do not
synchronize with other threads, but it is risky.

Of course this is exactly what startupHaskell does, though only in the case of multiple capabilities.

Use the memory management function from the dynamic C
Run-Time (CRT). If the CRT DLL is not initialized, calls
to these functions can cause the process to crash.

I expect the rts does this too.

Someone else should look this over but I suspect the thing to do is to change the user guide (section 11.6) to stop recommending calling startupHaskell inside DllMain and indeed to recommend never to do that.

comment:3 Changed 8 years ago by augustss

Perhaps calling startupHaskell is from DllMain() is not allowed, but this is what the ghc documentation suggests that you do.

comment:4 Changed 8 years ago by NeilMitchell

Component: CompilerDocumentation

Fixing the program to call startupHaskell after DllMain works perfectly. This is now a documentation bug (but a pretty severe one).

comment:5 Changed 8 years ago by igloo

difficulty: Unknown
Milestone: 6.12.1
Priority: normalhigh

Let's fix the docs for 6.12.1.

comment:6 Changed 8 years ago by simonmar

I had a nagging feeling I'd come across this before, and indeed the docs do have a section describing a lot of the problems with DllMain():

http://www.haskell.org/ghc/docs/latest/html/users_guide/win32-dlls.html#id585039

However earlier on in that section the sample code still contains a DllMain() that calls startupHaskell, so we should fix that. Also the section on DllMain talks more about shutdown than startup, but both are dangerous (fortunately the example code it gives is correct).

Neil, Lennart: could you check the docs I linked to above and see if you agree?

comment:7 Changed 8 years ago by NeilMitchell

I suggest the docs are rewritten to tell people never use DllMain, and just say that "calling either hs_init or hs_exit from DllMain may lead to program freezes - don't do it". That section also has two examples - one calling from C, and one from VBA. I suggest a complete rewrite showing how to create a single unified example DLL, then how to call it from both VBA and C++, never using DllMain in either case.

If that sounds satisfactory, I'll make the changes to the docs and send a patch in.

comment:8 Changed 8 years ago by simonmar

Sounds great to me. Thanks for offering to do this Neil!

comment:9 Changed 8 years ago by NeilMitchell

Type of failure: None/Unknown

I've written revised documentation and put it on my blog: http://neilmitchell.blogspot.com/2009/11/haskell-dlls-on-windows.html

After a short period of time I'll incorporate any comments, format it properly for the manual, and submit a patch.

I had a few questions:

Is calling hs_init or startupHaskell preferred? I've gone with hs_init because that's in the FFI spec, but the current examples have both (which is just confusing).

Section 11.6.1 (which I left alone) says "it will rewrite occurrence of -lHSfoo on the command line to -lHSfoo.dll". It's really not clear if it's rewriting -lfoo to -lfoo.dll, or -lHSfoo to -lHSfoo.dll - and similarly for the HScool line. Italics to indicate meta variables is very handy.

comment:1 in reply to:  9 Changed 8 years ago by duncan

Replying to NeilMitchell:

I had a few questions:

Is calling hs_init or startupHaskell preferred? I've gone with hs_init because that's in the FFI spec, but the current examples have both (which is just confusing).

I would use hs_init. The other one is (or should be) deprecated.

Section 11.6.1 (which I left alone) says "it will rewrite occurrence of -lHSfoo on the command line to -lHSfoo.dll". It's really not clear if it's rewriting -lfoo to -lfoo.dll, or -lHSfoo to -lHSfoo.dll - and similarly for the HScool line. Italics to indicate meta variables is very handy.

I would also check if this is actually true. I can't immediately see any evidence for it in the code.

comment:11 Changed 8 years ago by igloo

Milestone: 6.12.16.12.2

comment:12 Changed 7 years ago by igloo

Owner: set to NeilMitchell

comment:13 Changed 7 years ago by igloo

Milestone: 6.12.26.12.3

Just a doc improvement, and Neil says he won't have time to finish it in the 6.12.2 timeframe, so punting.

comment:14 Changed 7 years ago by igloo

Milestone: 6.12.36.14.1

comment:15 Changed 7 years ago by simonmar

Neil - if you have the patch for this but haven't been able to test it due to tool issues, then feel free to just attach it here and we'll try to incorporate it.

comment:16 Changed 7 years ago by NeilMitchell

Ok, will do - I'll make sure it's attached tomorrow morning.

Changed 7 years ago by NeilMitchell

Patch to the documentation

comment:17 Changed 7 years ago by NeilMitchell

Status: newpatch

Patch has been attached. Unfortunately I haven't been able to actually build and look at the documentation - I spent ages building on mingw only to find the documentation doesn't build on mingw, and I'm still going through contortions to get it building on cygwin. As a result, this patch is likely to have syntax errors.

comment:18 Changed 7 years ago by simonmar

Owner: changed from NeilMitchell to simonmar

Thanks Neil, I'll integrate it.

comment:19 Changed 7 years ago by simonmar

Status: patchmerge

Applied (with fixes to markup), thanks!

Sun Oct 10 03:07:09 PDT 2010  Neil Mitchell 
  * Update the documentation on using DLL's from Windows, fixing several errors, in particular those relating to bug 3605

Wed Oct 20 05:31:16 PDT 2010  Simon Marlow <marlowsd@gmail.com>
  * fix markup

comment:20 Changed 7 years ago by igloo

Resolution: fixed
Status: mergeclosed

Both merged.

Note: See TracTickets for help on using tickets.