View Issue Details

IDProjectCategoryView StatusLast Update
0001578OpenMPTGeneralpublic2022-03-10 21:17
ReporterWodd Assigned ToSaga Musix  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Platformx64OSWindowsOS Version10
Product VersionOpenMPT 1.31.00.* (current testing) 
Target VersionOpenMPT 1.30.03.00 / libopenmpt 0.6.2 (upgrade first)Fixed in VersionOpenMPT 1.30.03.00 / libopenmpt 0.6.2 (upgrade first) 
Summary0001578: Crashing While Navigating With Go To… Dialog
Description

I have found that OpenMPT appears to sometimes crash while navigating using the Go To… dialog.

Steps To Reproduce
  1. Open the module Has Paul Burkey Played Any of the Commander Keen Games Before¿ from https://onedrive.live.com/?id=BD6492FE45944E%21116&cid=00BD6492FE45944E . (Do note that it contains a total of 54,848 orders and patterns so you will need to temporarily increase the order and pattern limits in the code.).
  2. On the Patterns tab, navigate to pattern 14 (It might work with any pattern. I used 14.)
  3. Play the pattern (via Replay Pattern command).
  4. Open the Go To… dialog.
  5. Navigate to pattern 78.
  6. Repeat steps 4 and 5, incrementing the value in step 5 by 64 each time.
Additional Information

If it works as intended, eventually, you should hear the Windows Critical Stop sound (Windows Foreground.wav by default). Also, OpenMPT should become unresponsive. It might eventually display an error; however, I force terminated it before that occurred. I also performed the steps mentioned in the Steps To Reproduce section using 2 different methods, eventually yielding the same result. Those methods were:

  1. Press Enter immediately after typing the desired value in the Pattern field of the Go To… dialog
  2. Press Enter after waiting for the appropriate timestamp to be calculated. I do not know if the Go To… dialog’s calculating the timestamp is causing the crash or if it is unrelated. I cannot recall ever facing this issue prior to the timestamp being introduced into the Go To… dialog.
TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)17068

Activities

Wodd

Wodd

2022-03-10 09:20

reporter   ~0005129

Screenshot of error from Visual Studio 2022 (vs2022win10.sln): https://onedrive.live.com/?cid=00BD6492FE45944E&id=BD6492FE45944E%21613240&parId=BD6492FE45944E%21116&o=OneUp

Saga Musix

Saga Musix

2022-03-10 09:46

administrator   ~0005131

Last edited: 2022-03-10 09:50

Based on the screenshot, one thing you could try is adding #include "AudioCriticalSection.h" (minus the HTML garbage Mantis is inserting there) on line 29 of Snd_fx.cpp and then add CriticalSection cs; on line 1244 (before if(retval.targetReached || target.mode == GetLengthTarget::NoTarget)).

Wodd

Wodd

2022-03-10 10:48

reporter   ~0005132

I have tested the behavior in an attempt to identify the crash location as you had requested. However, it appears to be variable. Upon opening the module that I sent you the link to for testing, the crash occurred while navigating to pattern 718. However, after force-terminating the software, reopening it, reopening the module, and performing the same steps, the crash occurred upon attempting to navigate to pattern 78. I added the lines that you suggested and the crash is still present. So far, since adding the lines I have tested the behavior through the debugger twice. The exception that I took the screenshot of the first time was thrown in the first debugging session (while attempting to navigate from pattern 1,166 to pattern 1,230 [?] [It might have been while navigating from pattern 1,230 to pattern 1,294.]). However, the second session went further into the module and threw a different exception (while attempting to navigate from pattern 1,934 to pattern 1,998). Screenshot of the second exception: https://onedrive.live.com/?cid=00BD6492FE45944E&id=BD6492FE45944E%21613243&parId=BD6492FE45944E%21116&o=OneUp

Saga Musix

Saga Musix

2022-03-10 10:57

administrator   ~0005133

Right, the second line has to be placed before line 1242 instead.

Wodd

Wodd

2022-03-10 12:18

reporter   ~0005134

That appears to have fixed the issue. I did observe, however, that, sometimes, upon pressing Enter to navigate to the next target, the navigation fails to execute and it remains on the current pattern (example: While attempting to navigate from pattern 654 to pattern 718, the module remains on pattern 654 while the Go To… dialog closes as expected. Sadly, because that behavior does not throw any exceptions in the code, troubleshooting it might be much more difficult. That might be caused by pressing Enter too quickly and not allowing OpenMPT to properly calculate the time stamp, causing it to “not know where to go.” Because it “does not know where to go,” it does not move. From what I can see of it, however, the main issue of this thread appears to have been resolved by moving the lines that you mentioned up. I have not seen OpenMPT crash yet since that. I observed only the oddity of failed navigation. To try to reproduce the bizarre navigation behavior, you can try opening the Go To dialog (via keyboard shortcut), quickly press Tab 2 times, type a number in and press Enter. This has occurred with 2 tested modules so far, one of which being the one linked to in this thread (Has Paul Burkey Played Any of the Commander Keen Games Before¿). I know that this behavior should probably be treated as a separate thread. But:

  1. Some people greatly frown upon “double posting.”
  2. I figure that, since it is also related to the Go To dialog, I would at least mention it. A problem cannot even have an attempt at being corrected (even if the attempt is unsuccessful) if it is not known.
Saga Musix

Saga Musix

2022-03-10 21:17

administrator   ~0005135

r17094 fixes the crash in a slightly different way.

I did observe, however, that, sometimes, upon pressing Enter to navigate to the next target, the navigation fails to execute and it remains on the current pattern

I see no possibility how that could happen (and it definitely would not caused by this change), but you could add some debug logs in CPatternGotoDialog::OnOK to figure out if the dialog really jumps to the location you just typed in, and if you find a way to reproduce that, open a new issue (posting two unrelated things in the same issue is more akin to double-posting than creating separate issues for separate bugs).

Issue History

Date Modified Username Field Change
2022-03-10 03:50 Wodd New Issue
2022-03-10 05:11 Wodd Steps to Reproduce Updated
2022-03-10 09:20 Wodd Additional Information Updated
2022-03-10 09:20 Wodd Note Added: 0005129
2022-03-10 09:46 Saga Musix Note Added: 0005131
2022-03-10 09:50 Saga Musix Assigned To => Saga Musix
2022-03-10 09:50 Saga Musix Status new => assigned
2022-03-10 09:50 Saga Musix Note Edited: 0005131
2022-03-10 10:48 Wodd Note Added: 0005132
2022-03-10 10:57 Saga Musix Note Added: 0005133
2022-03-10 12:18 Wodd Note Added: 0005134
2022-03-10 21:17 Saga Musix Note Added: 0005135
2022-03-10 21:17 Saga Musix Status assigned => resolved
2022-03-10 21:17 Saga Musix Resolution open => fixed
2022-03-10 21:17 Saga Musix Fixed in Version => OpenMPT 1.30.03.00 / libopenmpt 0.6.2 (upgrade first)
2022-03-10 21:17 Saga Musix Target Version => OpenMPT 1.30.03.00 / libopenmpt 0.6.2 (upgrade first)