| | [ Return to Bugs & Features | Post Text | Post File | SVN ⇄ GIT | Prev | Next ]
STR #3350
| Application: | FLTK Library |
|---|
| Status: | 5 - New |
|---|
| Priority: | 1 - Request for Enhancement, e.g. asking for a feature |
|---|
| Scope: | 2 - Specific to an operating system |
|---|
| Subsystem: | DLL/DSO |
|---|
| Summary: | Some fix to handle dll multi injection and ejection |
|---|
| Version: | 1.4-feature |
|---|
| Created By: | AlainBandon |
|---|
| Assigned To: | Unassigned |
|---|
| Fix Version: | Unassigned |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
[ Post File ]
Trouble Report Comments:
[ Post Text ]
| |
| #1 | AlainBandon
03:39 Nov 06, 2016 |
| On windows, injecting a dll containing fltk multiple time in the same target process is currently not possible. This annoying problem is actually caused by the windows class register that register a classname in the main process. FLTK is using by default 2 class names : "FLTK" and "FLTimer", that are never unregistered when you eject your dll,making any other injection of any dll using fltk impossible.
I fixed this issue by writing the unregister class code and also making the lib able to be used multiple times in multiple dlls using the lib (like plugins for example) on the same target process, by creating unique class name versions given the module path name currently used.
Fill free to modify/adapt ;). (Sorry I didn't adopt the coding standards used in fltk) | |
| |
| #2 | AlbrechtS
04:47 Apr 03, 2017 |
| Moved to 1.4-feature.
@OP (AlainBandon: Did you intend to post a patch? If yes, please do so against FLTK 1.4, see 'branch-1.4'. TIA.
It would also be very helpful if you could post a simple self-contained (i.e. compileable) demo application and plugin so we can see the effect and test your proposed patch. Ideally the demo program would have buttons to load and unload your plugin. I'm afraid that we can't implement a solution for this special request without a patch and demo program. | |
| |
| #3 | heiko.bauer
02:31 Sep 14, 2017 |
| Hallo, what is the current status of this feature?
It would probably solve my problem. | |
| |
| #4 | greg.ercolano
08:36 Sep 15, 2017 |
| I'm updating this STR with the attachment Alain (Akabane87)
posted on fltk.general on Nov 5 2016, which I think is the one
he intended to include here.
I'll see if I can also turn this into a patch format diff
so we can see what's being changed. (This appears to be a
complete copy of the original file with modifications mixed in) | |
| |
| #5 | AlainBandon
09:03 Sep 15, 2017 |
| I just updated the file to fix a minor mistake I discovered recently in my code. The use of timer_class string instead of long_class_name when calling CreateWindowEx for the timer class, that was causing any use of win timers fail (like hints when hovering elements with the mouse). | |
| |
| #6 | greg.ercolano
09:21 Sep 15, 2017 |
| Attaching a patch version of Alain's changes; I pulled the svn
version of the file Alain had modified, r10387, and svn diff'ed it. | |
| |
| #7 | greg.ercolano
09:25 Sep 15, 2017 |
| Oh, and I see Alain just followed up with a _v2 version of this file.
So I'm supplying a new Fl_win32_v2.patch against r10387. | |
| |
| #8 | greg.ercolano
18:22 Sep 15, 2017 |
| Attaching Alain's v2 mods as a patch against 1.4.x, and posting here as:
Fl_win32_1.4.x_v2.patch
Tested these mods with VS7 only. Note: Alain, your v2 mods don't seem to
be needed in 1.4.x; the CreateWindow timer stuff was removed and replaced
with a wchar string that is already created from the long class name, so
I think that code is OK.
Fl_win32.cxx changed a lot since 1.3.x, but I think I merged it correctly.
I'd appreciate it if devs who worked most on Fl_win32.cxx since 1.3.x give it a check.
For instance, I did not check if Alain's patch has implications in the new 'driver' stuff.
I made some changes to Alain's code:
o static prefix on new functions (protects global namespace)
o replaced string sizes [100] and [200] with MAX_PATH
o sprintf_s not in VS7; used snprintf() instead (with NULL safeguards)
o CMP compliance
Comments welcome.
Heiko: I'm not sure what FLTK version you're using:
1.3.x -- try the Fl_win32_v2.patch
1.4.x -- try the Fl_win32_1.4.x_v2.patch | |
| |
| #9 | AlbrechtS
02:18 Sep 18, 2017 |
| @Heiko: what exactly is your problem? Can you describe in your own words what you want to achieve, how you do it, and how it fails?
@Heiko and @Alain:
Please, please, can one of you provide example code (a simple main program and a plugin), build instructions (preferred as a CMake file), and instructions how to run the example program (if not obvious).
This is very important so we can test without and with the patch to see what's happening when it fails and how it works with the modifications. I'd be willing to take a deeper look at the issue (when time permits), but I'm afraid this is not possible without a test case.
Note: Greg seems to be at least somewhat familiar with VS IDE setup, but I am not. I prefer to generate IDE solutions with CMake, and I hope one of you can provide CMakeLists.txt files. If not, please try to describe the project setup and the build as detailed as possible.
I'll follow up with a modified patch and some technical issues I found when I read the patch and did a first compile and build test.
@Greg: many thanks for doing the patch work (pun intended). | |
| |
| #10 | AlainBandon
03:04 Sep 18, 2017 |
| Unfortunatly I don't have any simple sample other than my full project. But I guess it's really straitforward to trigger the issue. You just have to do a basic sample as a dynamic library project without anything in particular (just normal init of fltk) in dllmain (maybe you'll have to start a thread doing this). Then load 2 instances of this dll in a process (you can do a dummy project that will only load the dll 2 times, and do an infinite while with a delay inside or just wait for input to close).
When loading the second dll, the init of fltk will fail (can't remember if it crashes or badly init), but the cause is that the main window class name and the timer class name are already registered.
So my fix's main idea is to create a unique name for main window class and timer for each dll.
It also makes sure to properly unregister the class name when exiting, allowing you to unload and then reload the same dll multiple times in the same process. | |
| |
| #11 | AlbrechtS
09:14 Sep 18, 2017 |
| Honestly, that's tough stuff in the context of the existing FLTK library. I don't really know how to start my comments. Unfortunately I won't have any more time to continue this during the next 10 days or so. So please don't expect quick replies.
First of all: FLTK documents how the window class is called (i.e. "FLTK") and lets you choose the class name in your application for each window with window->xclass(). This can not easily be changed w/o an additional API that lets the user set an option or something to switch to "long class names" as this is called in your patch. Maybe we can do that. The same is true for the (internal, hidden) timer window class, but maybe we can do this as well with the same or another option that would have to be set before Fl_Window::show() and before Fl::run(). Maybe...
I don't fully understand the unregister_class() stuff. When is it called? It appears to me that it is called when the dll is unloaded and thus the destructor of the NameList object is called. I see that the latter (destructor) is the case, but is it true that this happens when the dll is unloaded?
Then `EnumWindows(HandleWindowClosing, (LPARAM)(void*)classname);` which calls the callback function HandleWindowClosing() for all windows of the same class that are still open. How can it happen that the dll is unloaded (if my assumption above is true) while there are still windows open? Do you force the unload by your program?
In HandleWindowClosing() you call `DestroyWindow(hwnd);` which deletes the OS (Windows) window object w/o any interaction with FLTK. I assume the event loop was left somehow, the dll was unloaded somehow, but in the way you do it there are still windows open. How does this come?
There may be more questions, but let's stop here for now. Maybe you have some answers for me.
Follow-up: technical details... | |
| |
| #12 | AlainBandon
09:45 Sep 18, 2017 |
| Yes the unregister is called from the destructor when the dll is unloaded. This ensure all windows will have been closed before, what will be done by HandleWindowClosing in case some are still opened.
This, of course, will only happen in the case the dll is ejected externally (in case of a plugin needed to be reloaded on the fly for example). Otherwise I agree it should be the responsability of the dll to close fltk windows properly.
Honestly, the biggest issue there was the class name never unregistered. The rest of my "fixes" were added after to simplify the ejection process by making sure all is totally cleaned up even after an external dll ejection. | |
| |
| #13 | AlbrechtS
11:23 Sep 18, 2017 |
| [I'm still writing on the technical details...]
Thanks for you comments. More questions:
Why do we need the 'long class name' with the full path of the 'module', i.e. the loaded dll? Is it to make the class name unique? If yes, why? Is there a realistic chance that an application would load two or more FLTK-based plugins at the same time? If yes, does this work with your patch? How is it organized? In different threads? Again, if yes, do both plugins (threads) open their own windows?
I'll come back to the classname issue in my next comment. | |
| |
| #14 | AlbrechtS
11:43 Sep 18, 2017 |
| Sorry, this post is going to be very, very long:
Going forward through the code, adding comments as requested by Greg.
(1) #pragma comment( lib, "psapi.lib" )
This is Visual Studio specific and doesn't help on other platforms. I commented it out, since linking is the task of the build system, and such hints in the source code make development difficult ("why does it build with Visual Studio, but not with MinGW?"). Exactly that happened to me. [Side note: I know that we have some of these pragma's elsewhere in the source code, but I believe they should all be removed.]
(2) linking with Psapi.lib (dll): this is a new dependency, necessary for the new function GetModuleHandleByAddress() which calls EnumProcessModules() and GetModuleInformation(). Usually we'd dynamically load such dll's, get the function address from the dll, and call it via a function pointer - easy, done often. But not here. Citing the wonderful MS docs [1], slightly edited for readability in this context:
"Library
- Kernel32.lib on Windows 7 and Windows Server 2008 R2;
- Psapi.lib (if PSAPI_VERSION=1) on Windows 7 and Windows Server 2008 R2;
- Psapi.lib on Windows Server 2008, Windows Vista, Windows Server 2003, and Windows XP".
No mention of later Windows versions, but maybe I got the wrong documentation version. <irony> Questions? </irony>
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682631(v=vs.85).aspx
Note: we do still (at least, if possible) support WinXP, Win2000, and later. Rumors say that FLTK 1.3 still runs on Win98.
Anyway, if I didn't make a mistake, I built the FLTK demo progs on Windows 10 linked with Psapi.lib/dll and executed the resulting programs on my WinXP VM. Slightly surprised it worked...
(3) In GetModuleHandleByAddress():
while (1) {
DWORD cbNeeded = 0;
if (!EnumProcessModules(hProc, hmodules, cb, &cbNeeded)) {
return NULL;
}
if (cb >= cbNeeded) {
cb = cbNeeded;
break;
}
...
I feel uncomfortable with a while(1) loop, particularly if the code is not 100% clear in why it *will* terminate. I think it does, but it depends on the result of EnumProcessModules(). That's hard to read and to maintain. Shouldn't it be much easier with two function calls, like this (untested):
HMODULE *hmodules = NULL;
DWORD cb = 0;
DWORD cbNeeded = 0;
if (!EnumProcessModules(hProc, NULL, 0, &cbNeeded)) // get # of modules
return NULL;
cb = cbNeeded;
if (!EnumProcessModules(hProc, hmodules, cb, &cbNeeded)) // get modules
return NULL;
...
No loop, easy to read. Anything wrong with this?
(4) HandleWindowClosing():
#define USER_ERROR 0x20000000
static BOOL CALLBACK HandleWindowClosing(HWND hwnd, LPARAM lParam) { // str #3350
What is the constant USER_ERROR good for?
SetLastError(GetLastError() | USER_ERROR);
Why is SetLastError() called here?
(5) in unregister_class() and elsewhere:
We can't use the "ANSI" (*A) variants of functions like UnregisterClassA() and GetModuleFileNameA(). FLTK's internal strings *must* be UTF-8 encoded, but path's like in GetModuleFileName() can be arbitrary Windows Wide Characters (UTF-16). Hence we must always convert from the OS's view "Wide Character" (*W functions) to UTF-8 and back to be consistent.
Addmittedly, it "works" somehow even with the *A functions in the current code, but it is not correct and maybe not unique (if that was the intention).
Example: In my Cygwin shell (which is configured to display UTF-8 correctly and can handle UTF-8 (non-ASCII) filenames and directories) I added a subdirectory named
/umlaut-ä-ö-ü-€/
German Umlauts ä, ö, ü and the international Euro sign.
I put the FLTK clock.exe (built with MinGW) in that directory and executed it with some test (output) statements. You can see garbage at some places where the non-ASCII characters should be (I hope this will be transported correctly through our FLTK STR web page - if not, I'll likely post a pdf file later):
$ ls -ld /git/fltk-1.4/build/Debug/umlaut-ä-ö-ü-€/
drwxrwx---+ 1 0 Sep 18 19:28 /git/fltk-1.4/build/Debug/umlaut-ä-ö-ü-€/
$ /git/fltk-1.4/build/Debug/umlaut-ä-ö-ü-€/clock.exe
unregister_class('C:\git\fltk-1.4\build\Debug\umlaut-▒-▒-▒-▒\clock.exe_F
l_Clock')
$ /git/fltk-1.4/build/Debug/umlaut-ä-ö-ü-€/clock.exe 2>&1 | od -a -t x1
0000000 u n r e g i s t e r _ c l a s s
75 6e 72 65 67 69 73 74 65 72 5f 63 6c 61 73 73
0000020 ( ' C : \ g i t \ f l t k - 1 .
28 27 43 3a 5c 67 69 74 5c 66 6c 74 6b 2d 31 2e
0000040 4 \ b u i l d \ D e b u g \ u m
34 5c 62 75 69 6c 64 5c 44 65 62 75 67 5c 75 6d
0000060 l a u t - d - v - | - nul \ c l o
6c 61 75 74 2d e4 2d f6 2d fc 2d 80 5c 63 6c 6f
0000100 c k . e x e _ F l _ C l o c k '
63 6b 2e 65 78 65 5f 46 6c 5f 43 6c 6f 63 6b 27
0000120 ) cr nl
29 0d 0a
0000123
Note that `od -a` doesn't display characters with the 8th bit set correctly (in my setup), but you can get the idea from the hex (-t x1) part: the so-called 'ANSI' versions of the Umlaut characters are visible in the hex dump, but the € sign becomes 0x80, which is the first byte of its UTF-8 encoding (AFAICT off the top of my head).
That's it so far, but likely not complete.
Sorry, I must come to an end... | |
| |
| #15 | AlbrechtS
12:06 Sep 18, 2017 |
| Attached is my minimal patch (MinGW_and_CMake.patch) to make Fl_win32_1.4.x_v2.patch compile and link with MinGW (on FLTK 1.4.0). The build system is updated only for CMake (not autoconf/configure).
Apply it in branch-1.4 on top of Fl_win32_1.4.x_v2.patch with `patch -p1`.
Note: I did not change any other code mentioned in my comments above. | |
| |
| #16 | AlainBandon
00:34 Sep 19, 2017 |
| About your questions, yes this was an intent to make the plugin name unique in order to let the possibility to the user to use fltk independantly of other plugins. And yes it works in my project. The main program was using fltk in a separate thread and then another plugin was using it aswell in another thread.
For the rest I leave you guys handle this ;). | |
| |
| #17 | heiko.bauer
03:03 Sep 25, 2017 |
| Thank you guys and please excuse my late reply.
I develop a plugin for a quite big software using its API (https://3d-apps.faro-europe.com/product-category/scene-plug-in-apps/).
It works by dynamically loading/unloading of my dll.
The problem is the function call to CreateWindowExW. The whole program crashes completely whenever I do:
1. Start the plugin (load the dll)
2. Change something in my code and recompile it.
3. Remove the old plugin (unload the dll) and add the new plugin (reload the dll).
Unfortunately I cannot provide you any simple sample. But I can confirm, that the attached patch works for me. I applied Fl_win32_v2.patch on the official 1.3.4-1 release. | |
| |
| #18 | greg.ercolano
22:02 Sep 26, 2017 |
| Interesting: so I take it this is about hot-reloading plugins
in a continuously running FLTK oriented application..?
If so, yeah, this should probably at least end up being an article
if not a demo program, as I imagine there are a few apps out there
that would benefit from the ability to hot-swap dlls.
Probably none of the devs are very knowledgeable about this subject,
but assuming we 'do it right enough', we can write an article that
describes how this is supposed to work in a real app.
It's been a week or so since I looked at this (which means I've
already forgotten most of it) but I seem to recall looking at the code
this is more or less just a cleanup of how to handle names to be
more "fully qualified naming", by including /full pathnames/ instead of
just filenames.
It would be good to provide a demo though, including whatever separate
demo dlls are needed to test. | |
| |
| #19 | jcr6
11:52 Oct 17, 2017 |
| WooHoo!!! This solves my problem from 9/4/2012!
>>fltk Digest, Vol 104, Issue 3
>>DLL problem with Fl_Window->show() or Fl::run()
I was having a DLL re-entrancy problem (being unloaded and then reloaded with the UI not coming up). Basically it seemed like Fl::run() was never quitting.
My ugly work around was to have a stub DLL call a .exe (via command line and a memory-mapped file) built with FLTK to do the UI/processing. It was ugly, to say the least.
This solves it. Thank you, thank you, thank you.
-Chris | |
| |
| #20 | greg.ercolano
17:34 Oct 18, 2017 |
| Albrecht, how do you feel about a commit to 1.4.x?
Seems OK to me; small code mods, cleans up a long standing issue.
Perhaps too, we can apply to 1.3.x, just for completeness, since there
is a solution thanks to Alain's mods. | |
| |
| #21 | AlbrechtS
01:52 Oct 19, 2017 |
| @Greg: the patch in its current version is not acceptable to be committed, see my comment #14.
However, I think that the gist of it is correct, and since we have at least three confirmations that this patch works in the case of plugin (DLL) loading and unloading we should try to improve and commit it. My concern is mainly that it may affect other users and the handling of path names with the *a functions instead of *w with conversion to UTF-8.
I'll try to rewrite the critical portions and will post a modified version when I'm done with it. This will likely be during the next weekend.
Whether or not to commit to 1.3 as well: let's postpone this decision until we have a final version.
Assigning the STR to me (hope you don't mind). | |
| |
| #22 | AlbrechtS
18:52 Jan 01, 2018 |
| Progress report: I was able to build a small DLL with basically the FLTK hello program that opens a window and shows the text provided by the main (console) program that uses the DLL. I have a CMake file that currently supports MinGW and Visual Studio to build the plugin for testing. It's too late now to post my plugin code, but I will do this later.
I can execute the DLL function, the FLTK window opens, and after closing the FLTK window the DLL function exits, and I can start it again. This is all synchronous, no threads involved.
So far, so good. A new aspect (AFAICT not mentioned nor fixed with the supposed patch) is that the program crashes when I unload the plugin with FreeLibrary(). It crashes in FLTK's exit handler (a destructor of a static object) when OleUninitialize() is called (src/Fl_win32.cxx, line #565 in FLTK 1.4).
This is plausible, given Microsoft's comments about loading/unloading DLL's and using OLE functions that may load other DLL's. So I commented the call to OleUninitialize() out, and then unloading and reloading the plugin (DLL) works fine. This must likely be resolved by calling OleUninitialize() in another function *before* unloading the plugin.
If I modify the plugin after unloading and then load the new (modified) version the application crashes in unforeseeable ways. So I guess this is what the patch should fix. I'll investigate further...
After studying lots of Microsoft docs I assume that we can't call OleUninitialize() from DllMain() when unloading (i.e. "within" FreeLibrary()) but must use an extra call to unload or reset the FLTK lib *before* an application calls FreeLibrary() to unload the plugin. This should be doable by the plugin developer by calling a special FLTK function before unloading the plugin and could be supported by FLTK.
In my console (main) application I'm doing something similar to:
- LoadLibrary("fltk_plugin") - loads the plugin
- fltk_plugin_init() - currently empty, could initialize the class names and timer class
- fltk_plugin_exec() - opens a FLTK window and runs the event loop (normal plugin activity)
- fltk_plugin_unload() - currently empty, should call a new FLTK cleanup function (see below)
- FreeLibrary(fltk_plugin_handle) - unloads the plugin
Currently DllMain(), fltk_plugin_init() and fltk_plugin_unload() are empty functions. Note that these are function names specific to my plugin (called fltk_plugin), not functions in the FLTK library.
DllMain() can't do much (according to MS docs), particularly not if OleUninitialize() or other functions that may need other DLL's (that may already have been unloaded), are involved. Hence we need another function like fltk_plugin_unload() or maybe fltk_plugin_rundown() to do the cleanup work. If we had such a function we could also unregister the window classes etc. as proposed. That's why I prepared fltk_plugin_unload() which is still empty, but should call a new cleanup function in FLTK that does the cleanup work.
My proposal would be to have, for instance:
- fl_plugin_init( .. arguments .. )
- fl_plugin_rundown( .. arguments .. )
and documentation how to use these functions. They would be /optional/ for "normal" programs but would be required for plugins that can be unloaded and reloaded.
Questions to those that have the issues mentioned in this STR:
(1) can you imagine to add a call to fl_plugin_init() and fl_plugin_rundown() in your plugins? Note that fl_plugin_rundown *must* be called before the plugin is unloaded, ie. before FreeLibrary is called and NOT from within DllMain (the latter would be too late).
(2) Would this work for you? | |
| |
| #23 | AlbrechtS
19:04 Jan 01, 2018 |
| FYI: citing MS docs:
DllMain entry point
...
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.
Conversely, calling functions such as these during termination can cause
access violation errors because the corresponding component may already
have been unloaded or uninitialized.
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682583%28v=vs.85%29.aspx | |
| |
| #24 | AlbrechtS
07:08 Jan 02, 2018 |
| FYI: Funny (?), but as expected after all...
I wondered why the crash in OleUninitialize() didn't happen or was not reported by all the users with these DLL issues. Microsoft clearly states that the calls to OleInitialize() and OleUninitialize() must be balanced. Hence we have to call OleUninitialize() at exit, which is done in the d'tor of a static object: ~Fl_Win32_At_Exit(). But they also say that the concerned DLL's are only closed when their refcounts reach 0 (balanced calls).
So it seems that more complex programs than my very simple test case call OleInitialize() before a FLTK plugin can be (loaded and) unloaded. Hence the call to OleUninitialize() when the FLTK plugin is unloaded doesn't do any harm in "real programs" since it doesn't actually close the COM/OLE DLLs. Sigh.
To verify this assumption I added a call to OleInitialize() to my test program before the plugin(s) are loaded and OleUninitialize() at the end of the program (just to be "correct"), and it doesn't crash in OleUninitialize() any more. QED.
Although this is clear now (at least empirically verified) I'd still prefer to have a fl_plugin_rundown() function in FLTK that can be called before a plugin is unloaded.
Investigating and testing further... | |
| |
| #25 | heiko.bauer
23:48 Jan 08, 2018 |
| Hallo Albrecht.
calling fl_plugin_init() and fl_plugin_rundown() would be fine for me.
Thank you for your work. | |
| |
| #26 | heiko.bauer
00:49 Sep 19, 2018 |
| Hallo Albrecht,
any news about this issue? It would be nice to replace my patched version with an official release.
There is a vcpkg package with version 1.3.4-5. Is this issue fixed in this release?
Thank you | |
| |
| #27 | AlbrechtS
06:33 Sep 19, 2018 |
| Sorry, no progress. We're in the process of moving to git and this binds (particularly my own) resources.
What is a vcpkg? https://github.com/Microsoft/vcpkg.git ?
This is nothing official, so you'd ask the producer/maintainer of this vcpkg about it.
That said, just for my own curiosity: can you give us some more details about this vcpkg? | |
| |
| #28 | AlbrechtS
14:25 Jan 16, 2023 |
| Unassigning myself for now. Someone else may pick this, or I may take a look later, after the release of 1.4.0 | |
[ Return to Bugs & Features | Post Text | Post File ]
|
| |
