View Issue Details

IDProjectCategoryView StatusLast Update
0001126OpenMPT[All Projects] Accessibilitypublic2018-06-07 09:11
ReporterherodotasAssigned To 
PriorityhighSeverityblockReproducibilityalways
Status confirmedResolutionopen 
Platformx64OSWindowsOS Version7
Product VersionOpenMPT 1.28.00.* (current testing) 
Target VersionOpenMPT 1.28.01.00 (goals)Fixed in Version 
Summary0001126: Using DDE to open files from the shell when no OpenMPT instance is running is problematic
Description

When loading archived module, just getting annoying error message. It is not critical, module loading and playing normally. Picture included.

TagsNo tags attached.
Has the bug occurred in previous versions?No, just in 1.28.*
Tested code revision (in case you know it)

Activities

herodotas

herodotas

2018-06-01 17:07

reporter  

mptm.jpg (323,001 bytes)
Saga Musix

Saga Musix

2018-06-01 17:09

administrator   ~0003540

This message comes from Windows, not OpenMPT. I also noticed it starting to happen recently, starting at about the same time as a Windows update happened. Something in Windows' DDE handling might have changed, because I don't think there were any changes in the early OpenMPT setup code that would be run until this message is shown.

herodotas

herodotas

2018-06-01 17:16

reporter   ~0003541

But with 1.27.08 this message dont appear.

Saga Musix

Saga Musix

2018-06-01 17:23

administrator   ~0003542

Last edited: 2018-06-01 18:23

View 2 revisions

Seems to have started somewhere between r10065 (exclusive) and r10159 (inclusive). revisions inbetween are sadly not available on buildbot.

Likely candidate: GDI+ image support was added in r10067 and may cause initialization to take longer than what Windows is willing to wait for.

Saga Musix

Saga Musix

2018-06-01 22:02

administrator   ~0003543

As I suspected, GDI+ initialization might be at least part of the issue. Starting from r10077, we use GDI+ and that's where the issue started happening. The issue is not new (it could happen at random before), it is just more pronounced when having to load GDI+. We might have to create a window that accepts DDE messages way earlier in the intialization code.

manx

manx

2018-06-02 06:14

administrator   ~0003544

Creating anything DDE accepting earlier cannot possibly react to DDE messages earlier unless we actually start a thread doing that (which would open another can of worms, like the requirement to marshal DDE requests back to the main thread (probably SendMessage) or deal with initialization race conditions (blocking on SendMessage during initialization) or dealing with windows going bonkers because the first window we create is not in the main thread). The main thread is busy initializing ourselves and cannot react to any DDE message before it actually starts running the message loop.
The solution may be to completely stop using DDE because it is broken.

https://blogs.msdn.microsoft.com/oldnewthing/20070226-00/?p=27863
https://blogs.msdn.microsoft.com/oldnewthing/20100503-00/?p=14183
https://msdn.microsoft.com/en-us/library/cc144175.aspx

Sadly, IDropTarget also comes with some amount of development cost and its own can of worms (maintaining a correctly behaving COM local server). If we do not want that, we could also roll our own solution (either FindWindow+SendMessage to a running instance, or registering named synchronization facilities for communication with the running instance (a named pipe for example).
FindWindow is probably easier to implement, and a named pipe may be easier to get close to race-free.

There are 2 aspects to the whole problem:

  1. The way the shell tries to to open files:

    • DDE
    • IDropTarget in shell process
    • IDropTarget local server
    • custom commandline argument to the main exe
    • custom commandline argument to a launcher exe
  2. The way the list of files to open is transferred to the running instance:

    • DDE in main thread
    • DDE in separate thread (communicate with main thread via SendMessage)
    • IDropTarget local server
    • FindWindow/SendMessage to main window
    • FindWindow/SendMessage to handler window in separate thread (communicate with main thread via SendMessage)
    • named pipe polled in main thread
    • named pipe reading in separate thread (communicate with main thread via SendMessage)

I have no idea whatsoever which solution would be the best. This needs further discussion.

Saga Musix

Saga Musix

2018-06-02 10:54

administrator   ~0003545

Either way, the current behaviour is not acceptable and for me is a blocker for an 1.28 release, not a long-term goal.

manx

manx

2018-06-02 11:00

administrator   ~0003546

Fair enough, I just do not think this would be easily solvable.

manx

manx

2018-06-07 09:11

administrator   ~0003547

Regarding all options involving finding the main window from a separate process via FindWindow and sending file open command via SendMessage: We cannot actually easily use the main window for this, because MFC registers the main window class with some internal MFC window class name that is not really useful. Overriding this class name is possible, but feels very hack-ish and unsupported:
https://www.codeproject.com/Articles/196354/Provide-Your-Custom-Class-Name-to-your-MFC-Applica
https://stackoverflow.com/questions/29746215/mfc-random-name-and-class-name-of-main-window-process

So, this probably demands a hidden message-only window used specifically for IPC.

Issue History

Date Modified Username Field Change
2018-06-01 17:07 herodotas New Issue
2018-06-01 17:07 herodotas File Added: mptm.jpg
2018-06-01 17:09 Saga Musix Note Added: 0003540
2018-06-01 17:16 herodotas Note Added: 0003541
2018-06-01 17:23 Saga Musix Note Added: 0003542
2018-06-01 18:23 Saga Musix Note Edited: 0003542 View Revisions
2018-06-01 22:02 Saga Musix Note Added: 0003543
2018-06-02 06:14 manx Note Added: 0003544
2018-06-02 06:15 manx Status new => confirmed
2018-06-02 06:16 manx Target Version => OpenMPT 1.?? (long term goals)
2018-06-02 06:17 manx Severity minor => major
2018-06-02 06:17 manx Summary Annoying error message => Using DDE to open files from the shell into the same runing OpenMPT instane is problematic
2018-06-02 10:54 Saga Musix Note Added: 0003545
2018-06-02 11:00 manx Note Added: 0003546
2018-06-02 11:01 manx Priority normal => high
2018-06-02 11:01 manx Severity major => block
2018-06-02 11:01 manx Target Version OpenMPT 1.?? (long term goals) => OpenMPT 1.28.01.00 (goals)
2018-06-02 11:19 manx Summary Using DDE to open files from the shell into the same runing OpenMPT instane is problematic => Using DDE to open files from the shell into the same runing OpenMPT instance is problematic
2018-06-03 14:31 Saga Musix Summary Using DDE to open files from the shell into the same runing OpenMPT instance is problematic => Using DDE to open files from the shell when no OpenMPT instance is running is problematic
2018-06-07 09:11 manx Note Added: 0003547