Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003401Doomseeker[All Projects] Bugpublic2018-03-05 22:592019-04-22 06:24
ReporterZalewa 
Assigned ToPol M 
PrioritylowSeveritymajorReproducibilityalways
StatusassignedResolutionopen 
PlatformMicrosoftOSWindowsOS VersionXP/Vista/7
Product Version 
Target VersionFixed in Version 
Summary0003401: DOOMSEEKER_ABI mismatching plugins still pop up unknown procedure errors.
DescriptionDOOMSEEKER_ABI is meant to gracefully handle plugins with mismatching ABI. We bump the ABI, and plugins with mismatching ABI shouldn't load. Instead, plugins with mismatching ABI will still pop up errors on Windows. These errors are probably generated by LoadLibrary(), though more info should be gathered on that.
Steps To Reproduce1. Windows 7.
2. Change ABI, for example change Player(params) constructor.
3. Bump DOOMSEEKER_ABI definition.
4. Build only "doomseeker" target.
5. Try to run "doomseeker.exe".
6. Get spammed with popups telling you that Player(params) procedure is unknown - one popup for each plugin.
Attached Files

- Relationships

-  Notes
User avatar (0020461)
Pol M (developer)
2019-03-31 15:46
edited on: 2019-03-31 15:59

Looks like right now it only shows two similar error messages. I'm gonna disable one of them, and leave the other. Following the "steps to reproduce", I have not managed to get said results.
pr

User avatar (0020470)
Zalewa (developer)
2019-04-03 11:07

The pr is not enough as the problem is directly related to Windows loading the DLL and auto-generating the pop-ups. The displayed errors are not generated by us. I don't even know if this is solvable without parsing the DLL's import table before passing them to LoadLibrary() but maybe WinAPI provides some method of capturing those rough Windows errors and handling them gracefully.
User avatar (0020471)
Pol M (developer)
2019-04-03 13:14

Oh! That explains why on wine it was working. I'll investigate this issue on a native windows installation.
User avatar (0020496)
Pol M (developer)
2019-04-16 18:38

Okay, I've found an easy way to handle this: SetErrorMode
Unfortunately it won't allow us to know what errors are we suppressing. That said, since we can disable exclusively errors related to the engines, I'm not that worried about suppressing important info.
PR
User avatar (0020514)
Zalewa (developer)
2019-04-20 06:55
edited on: 2019-04-20 06:56

Oh the woes of see plus plus. The commit that disables SEM_FAILCRITICALERRORS does get rid of the invasive popup dialog boxes indeed. However, the
#define DOOMSEEKER_ABI_VERSION
still remains useless on Windows.

Let's see: a DLL with an incompatible ABI refuses to load, right? If it refuses to load, then we cannot compare the ABI in the DLL to the #define in Doomseeker. Therefore, the #define has no chance to do anything. However, if you think about it, does it really need to? Let's consider:

1. Since Windows already detects ABI incompatibility for us we already get the "feature" of an incompatible DLL not being loaded.

2. Since DLLs are not loaded we're already safe from incompatibilities causing crashes.

3. The obnoxious error popups are we gone. There's now a somewhat cryptic "error 127" in the logs, which is not so bad for programmers, but most users won't know what it means. We could improve that a bit by checking if GetLastError() returns ERROR_PROC_NOT_FOUND and print a better log message that directly states that the plugin is not in the correct version.

4. With the unusable #define what we can't do is to display Doomseeker's and plugin's ABI version numbers so that the user knows which one is newer. But, how useful would that be, really?

User avatar (0020515)
Zalewa (developer)
2019-04-20 07:26

I merged the PR as the commit here as it is a step in the right direction.
User avatar (0020517)
Pol M (developer)
2019-04-20 11:53

Quote from Zalewa

We could improve that a bit by checking if GetLastError() returns ERROR_PROC_NOT_FOUND and print a better log message that directly states that the plugin is not in the correct version.

That's an excellent idea. I'd manually add an exception to code 127 and add something like "perhaps this is an old engine? Check for newer updates!" (Something that kinda states what the problem could be and how to solve it)

Quote from Zalewa

 what we can't do is to display Doomseeker's and plugin's ABI version numbers so that the user knows which one is newer. But, how useful would that be, really?

While it's true that it would be interesting to show the info, the end user will solve the problem by updating anyway, so I'd say that it is not that bad that they lose that info. I find it highly unlikely that someone would downgrade to an older version of doomseeker and use newer engines, so the issue will almost always appear when people update doomseeker partially.
User avatar (0020520)
Zalewa (developer)
2019-04-21 13:27

The actual problems with ABI incompatibility will arise if someone decides to release a plugin that will be hosted outside of Doomseeker's source control and Doomseeker's release cycle. In such case it may be useful for the user to know if plugin is for an older or a newer version of Doomseeker.

If such plugin appears we will also need to be much more rigorous about Doomseeker's ABI instead of breaking it freely whenever the current Plugin API gets in the way of the development.
User avatar (0020524)
Pol M (developer)
2019-04-21 16:05

Quote from Zalewa

In such case it may be useful for the user to know if plugin is for an older or a newer version of Doomseeker.

Yeah, that will be a small problem. That said, I'd imagine that the developer will list the compatible doomseeker versions, and the slow release cycle will help us here, giving said developer time to adjust to the new ABI or mark the new Doomseeker version as compatible. It's a shame that the info will not be displayed, because it's very likely that they will use Windows to develop plugins. To mitigate a little the problem for developers (since the way I see it are the most afected) and maybe even players, we could say what the ABI is in the "About doomseeker" section, maybe at the left of the version? Like "1.3 (ABI: 3)". That would allow said developer to know very easily if the plugin needs a new version.
Quote from Zalewa

 will also need to be much more rigorous about Doomseeker's ABI

No problem. If the API gets on the way, we'll be more careful. Ticket 0001961 will do so (I have not been careful at all with ABI compatibility, so my guess is that it will).
User avatar (0020528)
Zalewa (developer)
2019-04-22 06:24

Displaying the ABI in the about dialog is not a bad idea.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2018-03-05 22:59 Zalewa New Issue
2018-10-05 07:22 WubTheCaptain Priority normal => low
2019-03-31 15:43 Pol M Assigned To => Pol M
2019-03-31 15:43 Pol M Status new => assigned
2019-03-31 15:46 Pol M Note Added: 0020461
2019-03-31 15:59 Pol M Note Edited: 0020461 View Revisions
2019-03-31 15:59 Pol M Status assigned => needs testing
2019-04-03 11:07 Zalewa Note Added: 0020470
2019-04-03 13:14 Pol M Note Added: 0020471
2019-04-03 13:14 Pol M Status needs testing => assigned
2019-04-16 18:38 Pol M Note Added: 0020496
2019-04-16 18:39 Pol M Status assigned => needs testing
2019-04-20 06:55 Zalewa Note Added: 0020514
2019-04-20 06:55 Zalewa Note Edited: 0020514 View Revisions
2019-04-20 06:56 Zalewa Note Edited: 0020514 View Revisions
2019-04-20 07:26 Zalewa Note Added: 0020515
2019-04-20 07:27 Zalewa Status needs testing => feedback
2019-04-20 11:53 Pol M Note Added: 0020517
2019-04-21 13:27 Zalewa Note Added: 0020520
2019-04-21 13:27 Zalewa Status feedback => assigned
2019-04-21 16:05 Pol M Note Added: 0020524
2019-04-22 06:24 Zalewa Note Added: 0020528






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker