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

Duplication error (0.6.0-rc3: Merge pull request #4 from korfuri/windows) #5

Closed
Ghalebor opened this issue Apr 20, 2019 · 8 comments
Closed

Comments

@Ghalebor
Copy link

Ok I provide little test on my system Win10 pro (AMD FX-8320E 8GB Ram, AMD Radeon R7 250x graphics)
Testing on VCV Rack 0.6.2c (vanila) and modded version Rcomian (v0.6.2c-experiments)
2019-04-20

In both cases "vanila" and "moded" version of VCV Rack:

  1. duplication works when "Cycle trough presets" is on "no"
  2. when I have single Milkrack module in rack chaanging "Cycle trough presets" to "yes" works ok
  3. duplication Milkrack module with "Cycle trough presets" is "yes" -> crash VCV Rack
  4. changing "Cycle trough presets" to "yes" with multiply instances of Milkrack -> Crash
@dizzisound
Copy link

It's a segmentation fault error raised in multi-threaded context at this point in code.
@korfuri Do you experience the same error in your system?
Mac users?

@korfuri
Copy link
Owner

korfuri commented Apr 21, 2019

I can reproduce on Linux. What's more, people have reported similar crashes with other software built on projectM including the VLC integration (https://forum.videolan.org/viewtopic.php?t=102176). This sounds like projectM just can't handle running multiple instances safely. I'm able to run multiple instances but it's inconsistent whether it works or not, and I've seen it crash in two other circumstances:

  1. Destroying an instance after cloning one causes the UI thread in Rack to hang forever. I can't seem to reproduce this consistently.
  2. This one is tricky but can be reproduced this way: create two instances, delete one. Enable preset autoplay on the remaining one. Upon preset transition, Milkrack segfaults.

I don't know what's the best way to proceed here. I see a few options:

  • Disallow running multiple Milkrack instances. Not great, as it breaks the VCV paradigm, and I'm not sure there's even a way to do this.
  • If multiple Milkrack instances are running, they share the same projectM instance. Only one would be used to acquire sound, the other instances' inputs would be ignored. Not great either, and I can't think of a way to convey that this is intended behavior to users.
  • Fix projectM itself. I haven't looked into this enough yet but my hunch is that it's related to projectM's internal thread management. There appears to be a way to disable that, I haven't tried it but I'll spend time tomorrow working on that option.

Let me know if you see other ways to fix this.

@dizzisound
Copy link

I think you just covered all possibilities. Unless you count the chance of removing the Cycle through presets option, that's the major factor triggering the exceptions, if I see it clear (because I can't reproduce the use-case you described in bullet 1 above, if I have the option unchecked).

korfuri added a commit that referenced this issue Apr 21, 2019
This means that projectM stuff will run in the UI thread instead of
its own thread. It appears to solve the odd issues related to module
duplication reported in #5.
@korfuri
Copy link
Owner

korfuri commented Apr 21, 2019

Good news, I had success with disabling projectM's threading. I have not actually really looked at how threading works inside projectM, I figured I'd just compile it without threading support and see, and what do you know, it seemed to work right away (I at least expected I'd have to manage threads myself or something). I haven't been able to cause any crash or hanging in this configuration, either with or without autoplay enabled. That is very encouraging :)

I also realized that early in my testing I had found that deleting the projectM instance was causing issue, so I had punted that to later and had left a gaping memory leak by not deleting the instance when the module was deleted. This is now fixed on master.

@dizzisound can you rebuild projectM on windows with --disable-threading (don't forget to make clean in the projectm folder first) and upload a new prebuilt lib?

I've also tried to reproduce on OSX but got blocked by #2.

@korfuri
Copy link
Owner

korfuri commented Apr 21, 2019

I built OSX and Linux RCs with --disable-threading, if people want to give that a try: https://github.com/korfuri/Milkrack/releases/tag/0.6.0-rc5

@dizzisound
Copy link

Nice. I will surely try and rebuild, only thing I won't be at my dev machine until Tuesday, sorry for the delay!

@dizzisound
Copy link

@korfuri said:
@dizzisound can you rebuild projectM on windows with --disable-threading (don't forget to make clean in the projectm folder first) and upload a new prebuilt lib?

@korfuri Hi, Uriel. I just got it done and pushed in my forked repo:
https://github.com/dizzisound/Milkrack/tree/master/libs/win/libprojectM

Tested on Windows 8.1 x64. It's working smoothly, with no bug 👍
Next days I'll try to figure out a way to normalize my build sources for libprojectM and push it somewhere in my Github.

@korfuri
Copy link
Owner

korfuri commented Apr 24, 2019

Thanks, I merged this in 524d377.
I have pending changes in branch glfw-separate-context and with those changes things seem to work on OSX wit threading disabled as well, so I'm going to mark this issue as fixed. Thanks!

@korfuri korfuri closed this as completed Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants