AuburnSounds / Dplug

Audio plugin framework. VST2/VST3/AU/AAX/LV2 for Linux/macOS/Windows.
https://dplug.org/
Other
484 stars 32 forks source link

Crash 0SX + (Cubase or Logic or Digital Performer) #110

Closed p0nce closed 8 years ago

p0nce commented 8 years ago

The VST dispatcher throws an Error somewhere. To reproduce, attach and instantiate.

p0nce commented 8 years ago

Already 3 Cubase reporters

p0nce commented 8 years ago

Looks like a race condition inside createGraphicsLazily. It looks like effEditGetRect and effEditOpen are called simultaneously.

p0nce commented 8 years ago

Should be fixed with fbfa03cfc12e64e4a6fb087d3782c7973d4b6ca8

p0nce commented 8 years ago

Users say it was a bad fix

p0nce commented 8 years ago

To reproduce use small buffers like 32 samples and open/close lots of Panagement. At the other side another thread is waiting on a semaphore.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.auburnsounds.vst.Panagement 0x0000000116ffbb24 _D5dplug6client6client6Client8closeGUIMFZv + 4

p0nce commented 8 years ago

Could be a race between audio processing and creating the widgets that report something graphically... (edit: this one seems fixed for good)

p0nce commented 8 years ago

Got into trouble with shared https://issues.dlang.org/show_bug.cgi?id=16277

p0nce commented 8 years ago

Removed two more race conditions wrt to DSP => UI interaction but this is something else.

p0nce commented 8 years ago

The remaining bug is that sometimes drawRect throws in Cocoa window. To reproduce, open/close lots of times the same plugin. This is quite long to trigger, maybe not Cubase specific.

EDIT: unfortunately much more common for some testers

p0nce commented 8 years ago

Possibly reproduced under Windows using the new stress-plugin program. Looks like the GC is collecting the IWindow object instance for no reason.

p0nce commented 8 years ago

Commenter on Gearslutz say it also crashes Cubase on Windows. (EDIT: reproduced with Panagement 1.0, the bug has since be fixed in dplug, must be the race conditions afore-mentionned.

p0nce commented 8 years ago

stress-plugin does not reproduce anything even though it has been created with that purpose. Great.

p0nce commented 8 years ago

Bug still here, look like it happens after the same number of instantiation

p0nce commented 8 years ago

Does it happen with other another host? Does it happen in Carbon?

p0nce commented 8 years ago

Panagement IGraphics creation fails when called from eddEditRect, not a race

p0nce commented 8 years ago

Faster to reproduce (5 instantiations) if breaking on C++ exceptions echo "break set -E C++" > .lldbinit + dispatcher logging enabled, even though it does not catch the bug

p0nce commented 8 years ago

Pretty sure the remaining bug is related to the GC collecting something. If we stress the GC the bug triggers outside of effEditGetRect.

p0nce commented 8 years ago

Is probably an out-of-memory (Windows), GC collect does not collect some garbage... (edit: no, was DMD specific).

p0nce commented 8 years ago

GarageBand + AU 32-bit also has a GC crash, is probably the same.

p0nce commented 8 years ago

What happens is that the dispatcher thread in AU or VST trigger the GC (with PNG decoding) and that causes a full collect. But another thread is not suspendable and the GC crash in suspendAll.

p0nce commented 8 years ago

Can this be workarounded? Possible work-around: disable the GC on OSX. But that require removing GC uses afterwards, such as in image decoding or font glyph caches. Last time we disabled GC, it wasn't great either. (edit: sounds impractical)

p0nce commented 8 years ago

Is this because the GC suspends a high-priority thread? (EDIT: no, not only)

Yes, if we don't register the thread coming with selector 14 in AU there is no problem. That doesn't explain the problem with Cubase though. Unless Cubase uses the same high-priority thread for everything :(

p0nce commented 8 years ago

~~We should ensure no collection occurs, while space-leaks are reduced to a minimum. Fortunately not much is allocating.~~ (edit: completely impractical) onDraw methods can probably made @nogc.

p0nce commented 8 years ago

Cubase does not support AU so the problem can be solved trivially for AU. For Cubase+VST it isn't practical to avoid GC completely though.

p0nce commented 8 years ago

Action: modify VST and AU logging so that we see thread IDs in both audio and dispatchers, will help future debugging. (edit: OK. Cubase does use a thread for audio and another for UI).

p0nce commented 8 years ago

Detaching the threads avoids the thread suspend bug in Cubase, and uncovers another bug: Pangement GUI GC-proof resource has leaked, and is destroyed on the first full collection after closing the first instance. Which is strange since if effClose has been sent, should be .destroyed.

p0nce commented 8 years ago

Action: try to finalize the D runtime to catch unclosed resources (effClose or LDC global destructors).

p0nce commented 8 years ago

What happens reliably on 2nd instantiation in Cubase + OSX is that in PanagementGUI constructor the GUIGraphics object is collected by the GC cycle triggered by image decoding. Very sad!

EDIT: More precisely PanagementGUI is collected while in its constructor, which must means it wasn't reachable.

p0nce commented 8 years ago

Can reproduce the same thing in Windows + stress-plugin64 + VST when detaching threads after callbacks.

p0nce commented 8 years ago

Seems gone now. The problem was rogue thread_detachThis when the corresponding thread_attachThis was doing nothing. On OSX + VST every thread can be detached, but the main thread (coming to VSTPluginMain) is an exception and isn't detached. TODO test with this thread also detached. => OK, no more problem in Cubase TODO quantify the slowdown which might be considerable for UI timers.(EDIT: no, it's OK)

p0nce commented 8 years ago

On Windows it seems workable to detach threads too. Audio processing then go unexplicably a bit faster. (edit: no, no appreciable speed differences in both the UI and audio department)

p0nce commented 8 years ago

Closed with a731376