Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid min/max value on components.Pot causes segfault #14308

Open
SamWhited opened this issue Feb 7, 2025 · 30 comments
Open

Invalid min/max value on components.Pot causes segfault #14308

SamWhited opened this issue Feb 7, 2025 · 30 comments
Labels

Comments

@SamWhited
Copy link

SamWhited commented Feb 7, 2025

Bug Description

While wiring up the pitchbend wheel on my keyboard I ran into a crash. I wired it up like so (which is probably wrong, I'm not 100% sure what the values are yet, but I didn't expect a segfault either way):

let pitchWheel = new components.Pot({
	group: "[Channel1]",
	midi: [0xE0 + midiChannel, 0x00],
	inKey: "pitch_adjust",
	output: undefined,
	max: 3,
	min: -3,
});

And if a track is playing and I move the pitch bend wheel it stops playing, Mixxx freezes up, and then segfaults. If I comment out the "max" line everything appears to work. The logs show the following:

 controller.keylab88midi1.input:debug [Controller] "incoming: " "KeyLab 88 MIDI 1: t:57560 ms status 0xE0: pitch bend ch 1, value 0x2000"
… unrelated outgoing values trimmed …
RubberBand: WARNING: reconfigure(): window allocation required in realtime mode, size: 262144
warning [Main] VisualPlayPosition::calcOffsetAtNextVSync "[Channel1]" no transport (offset > maxOffset)
Segmentation fault (core dumped)

Version

2.5.0

OS

Arch Linux, kernel 6.13.1-arch1-1

@SamWhited SamWhited added the bug label Feb 7, 2025
@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 7, 2025

min does nothing. max is about what values the controller is being expected to send (eg 127 on an absolute 7-bit control). You're likely somehow producing huge values that cause some DSP logic to crash. Can you clarify what you mean by pitch bend wheel in this case? Are you using an actual Pot to change the values or just one/two key(s)? Have you looked at rate_temp_up/rate_temp_down and pitch_up/pitch_down?
Either way, if you experience a segfault, please also create a backtrace. Thank you.

@SamWhited
Copy link
Author

SamWhited commented Feb 7, 2025

It's a pitch bend wheel like you'd get on a synth. It goes between 0x0 on the low end, and 0x3FFF on the high end, and it re-centers itself at 0x2000 when you let go.

I tried to get a backtrace, but when I follow the instructions after paging through the output gdb closes and clears the buffer and there's nothing left to copy, I'm just back at the terminal where I ran gdb with no output to copy.
I tried again using GDB's logging feature, and that time it took so long dumping information about threads that GDB and the tmux session it was running in crashed as well. I attached as much as got saved.

gdb.txt

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

So I assume it sends two different messages, one with the LSB and one with the MSB? See https://github.com/mixxxdj/mixxx/wiki/components%20JS#pot on how to map that. You shouldn't need to touch max because the default should be correct for you (max is the maximum value of the hardware control, so 0x3FFF in your case). I'm fairly certain it crashed because you set it so low, causing mixxx to try and set huge pitch_adjust values which in turn caused rubberband to crash (there have been several bug reports on similar issues in regards to crashes due to unchecked rubberband parameters).

@SamWhited
Copy link
Author

I got it working once I realized max was for the hardware side, not the parameter side, still feels like a segfault is probably not the correct response to a bad parameter value though :)

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

yeah, fully agree. Thank you for the backtrace. It does give additional indication that the crash is in fact not a SIGSEGV but a SIGFPE, which would support my previous hypothesis. However it does not contain the stacktrace of the crashing thread. Unfortunately, I can't reproduce the issue locally so we'll need to dig a little deeper on your end to find the cause. Can you try to create another backtrace, but this time only get the backtrace from the crashing thread? just simply doing bt instead of thread apply all bt should to the trick and hopefully not crash gcc this time.

@SamWhited
Copy link
Author

Updated backtrace attached!

gdb.txt

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

Thank you. Interestingly, this time its a proper SIGSEGV, so the backtrace is slightly different from the incomplete one you posted previously. It does contain a crash, I couldn't tell how that relates to the pitch_adjust though and I also can't tell on first sight where thats coming from. I'm afraid if you wanna get to the bottom of it you'll have to debug it yourself.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

I will close this issue for now. Please file a new one for the (likely) unrelated crash.

@Swiftb0y Swiftb0y closed this as completed Feb 8, 2025
@SamWhited
Copy link
Author

SamWhited commented Feb 8, 2025

I'm not really sure what to file if this looks unrelated? It only happens after I twitch the wheel, and it happens consistently

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

Does it only happen with the broken code you posted initially? Can you log what value its trying to set? If you start mixxx with the dev tools (mixxx --developer) you can manually set the pitch_adjust to the same/similar value via the "Developer Tools" window (accessible from the "Developer" menu bar item or Ctrl+Shift+T). Does it still happen if you do that?

@SamWhited
Copy link
Author

The broken code I posted initially is the only way I've managed to reproduce it thus far (I did a similar thing with one of the knobs on the keyboard and it didn't happen though, so maybe it's something specific about the pitch wheel). It sets lots of values rapidly as you move it, but for example I see:

 controller.keylab88midi1.input:debug [Controller] "incoming: " "KeyLab 88 MIDI 1: t:37823 ms status 0xE0: pitch bend ch 1, value 0x2A80"

Right before the crash. It says "Segmentation fault" every single time. When I start the developer window [Channel1],pitch_adjust is set to "1". If I then set it to "10880" nothing happens (to the pitch of the playing song and there is no crash).

@SamWhited
Copy link
Author

SamWhited commented Feb 8, 2025

Oops, excuse me, that's not true, the item column was too narrow and that was a different pitch_adjust value. pitch_adjust has its value set to 0 and its parameter set to 0.5. if I set it to 10880 it does indeed crash.

EDIT: still sort of not true? It doesn't actually crash, but the CPU spins up, the music stops playing, and if I try to hit play it doesn't do anything. If I try to load another track it says "Loading track…" but never seems to do it, so apparently something is stuck instead of crashing. If I close Mixxx it never actually shuts down either, the UI closes but the process never terminates.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

EDIT: still sort of not true? It doesn't actually crash, but the CPU spins up, the music stops playing, and if I try to hit play it doesn't do anything. If I try to load another track it says "Loading track…" but never seems to do it, so apparently something is stuck instead of crashing. If I close Mixxx it never actually shuts down either, the UI closes but the process never terminates.

I see, so that looks like a plain infinite loop somewhere... If you do that and attach a debugger, you should be able to stop the offending thread and still generate a backtrace.

Another interesting note: your keyboard seems to produce proper midi pitch-bend values. I'm unsure how well mix handles those in general (I thought these were plain midi CC messages like the knobs).

@SamWhited
Copy link
Author

SamWhited commented Feb 8, 2025

If you do that and attach a debugger, you should be able to stop the offending thread and still generate a backtrace.

How would I do that? Can we re-open this issue? It seems plain that something is going on, and I'd hate for this to get lost and not be kept track of or fixed just because we're not sure exactly what's causing it.

@Swiftb0y Swiftb0y reopened this Feb 8, 2025
@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

@SamWhited
Copy link
Author

I'm not sure what they're asking me to do there, pstack doesn't appear to be a gdb command or a thing installed on my system (or available in my package manager as far as I could see). I tried making it freeze, then suspending Mixxx and running bt. Hopefully that helps?

gdb.txt

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

yes, thats basically what I was asking of you. Unfortunately this only contains a backtrace from the stopped thread (which is likely not the one where the infinite loop was occurring). So this time I'd need a thread apply all bt.

@SamWhited
Copy link
Author

Enclosed:

gdb.txt

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

This could be the offender:

Thread 130 (Thread 0x7ffe7dc1e6c0 (LWP 61555) "mixxx"):
#0  0x00007ffff796db19 in RubberBand::R2Stretcher::calculateSizes (this=this@entry=0x7fffa5855320) at ../rubberband-4.0.0/src/faster/R2Stretcher.cpp:449
#1  0x00007ffff797415c in RubberBand::R2Stretcher::reconfigure (this=this@entry=0x7fffa5855320) at ../rubberband-4.0.0/src/faster/R2Stretcher.cpp:746
#2  0x00007ffff7975699 in RubberBand::R2Stretcher::setPitchScale (this=0x7fffa5855320, fs=8.5874206154115044e+272) at ../rubberband-4.0.0/src/faster/R2Stretcher.cpp:271
#3  0x0000555555a4aeac in EngineBufferScaleRubberBand::setScaleParameters (this=0x555557a855b0, base_rate=1, pTempoRatio=0x7ffe7dc14e40, pPitchRatio=0x7ffe7dc14e48) at /usr/include/c++/14.2.1/bits/unique_ptr.h:193
#4  0x0000555555b620fd in EngineBuffer::processTrackLocked (this=this@entry=0x55555780acd0, pOutput=pOutput@entry=0x555557c5ef80, iBufferSize=iBufferSize@entry=2048, sampleRate=...) at /usr/src/debug/mixxx/mixxx-2.5.0/src/engine/enginebuffer.cpp:1026
#5  0x0000555555b64b28 in EngineBuffer::process (this=0x55555780acd0, pOutput=0x555557c5ef80, iBufferSize=2048) at /usr/src/debug/mixxx/mixxx-2.5.0/src/engine/enginebuffer.cpp:1183
#6  0x0000555555b4a137 in EngineDeck::process (this=0x555556b78410, pOut=0x555557c5ef80, iBufferSize=2048) at /usr/src/debug/mixxx/mixxx-2.5.0/src/engine/channels/enginedeck.cpp:64
#7  0x00005555557fa5f2 in EngineMixer::processChannels (this=0x555557374680, iBufferSize=<optimized out>) at /usr/src/debug/mixxx/mixxx-2.5.0/src/engine/enginemixer.cpp:367
#8  EngineMixer::process (this=0x555557374680, iBufferSize=<optimized out>) at /usr/src/debug/mixxx/mixxx-2.5.0/src/engine/enginemixer.cpp:426
#9  0x0000555555ce2dce in SoundManager::onDeviceOutputCallback (this=<optimized out>, iFramesPerBuffer=1024) at /usr/src/debug/mixxx/mixxx-2.5.0/src/soundio/soundmanager.cpp:596
#10 SoundDevicePortAudio::callbackProcessClkRef (this=0x5555574f6920, framesPerBuffer=1024, out=0x55559e093960, in=<optimized out>, timeInfo=<optimized out>, statusFlags=<optimized out>) at /usr/src/debug/mixxx/mixxx-2.5.0/src/soundio/sounddeviceportaudio.cpp:1026
#11 0x00007ffff7cd9d66 in NonAdaptingProcess (bp=bp@entry=0x555598067688, streamCallbackResult=streamCallbackResult@entry=0x7ffe7dc15408, hostInputChannels=0x555597ceb430, hostOutputChannels=0x55559ab52f70, framesToProcess=<optimized out>) at src/common/pa_process.c:873
#12 0x00007ffff7ce2daf in PaUtil_EndBufferProcessing (bp=bp@entry=0x555598067688, streamCallbackResult=streamCallbackResult@entry=0x7ffe7dc15408) at src/common/pa_process.c:1564
#13 0x00007ffff7ce6477 in CallbackThreadFunc (userData=0x555598067620) at src/hostapi/alsa/pa_linux_alsa.c:4366
#14 0x00007ffff10a370a in start_thread (arg=<optimized out>) at pthread_create.c:448
#15 0x00007ffff1127aac in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

From what I can tell, its using the R2 stretcher. You can try choosing the R3 stretcher in the settings and see if the behavior is different. Now that we know which thread is the offender, it would be nice to see all the variables of that stack to determine which parameter causes the issue.

@SamWhited
Copy link
Author

SamWhited commented Feb 8, 2025

You can try choosing the R3 stretcher in the settings and see if the behavior is different.

I'm assuming this is something to do with the Keylock/Pitch-Bending Engine option? I don't see anything that says R2 or R3, but in that menu I have "Rubberband (better)" and "Soundtouch (faster)". If I select soundtouch, hit okay, and then move the pitch wheel the same behavior happens.

Interestingly, the pitch wheel also appears to be causing it to freeze up now (before changing that setting), not crash. I have no idea what changed, as far as I know I didn't mess with any other settings.

Now that we know which thread is the offender, it would be nice to see all the variables of that stack to determine which parameter causes the issue.

If you can tell me how to do this, I'll see what I can do!

EDIT: the internet suggests that Mixxx only provides R2 and R3 on Windows and that on Linux the system version is used. I appear to have rubberband 4.0 installed:

$ pacman -Q rubberband
rubberband 4.0.0-1

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

the internet suggests that Mixxx only provides R2 and R3 on Windows and that on Linux the system version is used. I appear to have rubberband 4.0 installed:

Nah, that's BS afaik. The system version is used, but it ships both R2 and R3. You're likely suffering from the issue addressed in #14198 which is not yet in any release version. You either need to wait some time until we get a release out (and the arch ppl package it) or you compile the 2.5 branch yourself.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

Interestingly, the pitch wheel also appears to be causing it to freeze up now (before changing that setting), not crash. I have no idea what changed, as far as I know I didn't mess with any other settings.

Yeah, mixxx doesn't seem to be able to deal with 0xE0 messages properly as far as I can tell. I don't really have time to dig into that myself unfortunately right now...

@SamWhited
Copy link
Author

Quick update: I built Mixxx from the default branch (HEAD at adda147) and the problem persists there, so whatever it is it's not one of the fixed issues.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

Well, now that you've built HEAD, you should be able to select the R3 engine and see if the problem persists. If so, please upload another backtrace with the R3 engine.

@SamWhited
Copy link
Author

Ah yes, sorry, forgot the point was to test with R3. The problem does persist and I see this in the logs:

RubberBand: R3Stretcher: WARNING: Ratio yields ideal inhop < minimum, results may be suspect: (0.0078125, 1)

Trace attached.

gdb.txt

@SamWhited
Copy link
Author

SamWhited commented Feb 8, 2025

Here's the same thing but with thread apply all bt which is what I meant to do the first time.

gdb.txt

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

I think that trace is outdated (at least judging by the function names).

I went to go double check and found #8847 and #4354 which I thought we had merged already. Unfortunately the poster seems to have abandoned the PR. This is usually the point where I'd encourage you to adopt it, but since you don't like the CLA someone else will have to come along.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 8, 2025

summarizing: This is likely a duplicate of #8847

@SamWhited
Copy link
Author

Sounds similar; thanks for all your help with debugging this!

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 9, 2025

No worries, Thank you for your repeated patience to provide the backtraces.

@SamWhited SamWhited changed the title Invalid min/max on Invalid min/max value on components.Pot causes segfault Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants