rsjaffe / MIDI2LR

An application and plugin to remotely control Lightroom with a MIDI controller
http://rsjaffe.github.io/MIDI2LR/
GNU General Public License v3.0
698 stars 83 forks source link

Crash on OS X if app shut down directly #152

Closed StevenKersting closed 8 years ago

StevenKersting commented 8 years ago

I've spent several hours programming my Midi controller and the Midi2LR plugin. I have to say it's a bit overwhelming to figure out how to setup. At least for someone like me with zero Midi experience. BTW, is it typically necessary to program a controller button to output 127 for both high and low? Without that I was having to press buttons twice frequently.

BUT, once it's sorted it's hugely flexible/capable. I'm quite impressed. I have utilized the latest version with panel/profile switching on Mac to generate a single program that allows control of almost every option in a fairly intuitive way. I'll be setting up an import and sorting/rating program next. (I'll probably make a video of how I set it up and how it works with all of the latest features if anyone is interested).

Currently I have two glitches... the ability to turn on/off the tone curve panel seems to be missing.

But what I'm more concerned about is the shutdown errors I'm getting. If I shut down Midi2LR first I get an "unexpected stop" alert from LR. I suspect that's no big deal.

But regardless of how I shut things down LR always fails to shut down the server process correctly. I've only found it to cause one error so far, which was a user preset I created during the process and linked in the options was missing entirely when LR was restarted. I have no idea how concerned I should be about this failure to shut down... but at the very least it is annoying as the OS/App hangs while attempting it.

rsjaffe commented 8 years ago

Not having a Mac, I can't debug the failure to terminate the app. Is anyone else having this issue? With what version MIDI2LR?

We need to write better documentation on controller setup. While what you did works, it causes the button lights to stop corresponding to the "on" state. What we've recommended is that the button emit 127 for on and 0 for off, and the button set to "toggle off", which means that the button emits on when pressed and off when released. Your buttons are currently set to "toggle on", so each press of the button causes it to change state off to on or on to off, and the release of the button doesn't do anything.

StevenKersting commented 8 years ago

Thanks. Version 0.9.4

The buttons are configured to send 127 for both off and on in a latching state (toggle on). Does a signal of 0 do anything? My controller doesn't have any lights other than the LCD...

StevenKersting commented 8 years ago

Currently w/ 9.5.4, El Capitain, LR CC 2015.4

LR is able to shut down the server successfully... it's just slow (~20-30sec for LR to shut down completely). If I quit MIDI2LR from the Mac Dock or from the MIDI2LR menu it shuts down cleanly. But if I close MIDI2LR using the close button (red "x" in the header) I get the "quit unexpectedly" alert. The alert comes from the OS and not from LR. It happens even if I close LR before closing the app.

It causes no issues and I would consider it a minor bug. But it seems like something that should be an easy fix.

Jewest commented 8 years ago

The unexpected shut down I can confirm. In my test set up it's also crashing.

rsjaffe commented 8 years ago

is it doing this in OSX and Windows or just OSX. What directory is the plugin in and what are your privileges for that directory?

Thanks Rory

Jewest commented 8 years ago

I'll get back to you on that. Have to figure out how this works in apple os. Guessing that is should not be a problem as the code is stored in the documents tree. Xcode is giving exception, currently in the process of trying to read this correctly.

Regards,

Jeffrey Op 14 feb. 2016 23:05 schreef "rsjaffe" notifications@github.com:

is it doing this in OSX and Windows or just OSX. What directory is the plugin in and what are your privileges for that directory?

Thanks Rory

— Reply to this email directly or view it on GitHub https://github.com/rsjaffe/MIDI2LR/issues/152#issuecomment-183989951.

rsjaffe commented 8 years ago

I think I addressed the crash issue. See https://github.com/rsjaffe/MIDI2LR/blob/master/Source/Main.cpp . The shutdown process when the system requested shutdown bypassed deleting the window and closing down the IPC sockets--I changed that. Haven't had a chance to test yet.

StevenKersting commented 8 years ago

Crash still occurs w/ 9.7, 9.8

rsjaffe commented 8 years ago

If anyone with a Mac wants to debug the crash issue, I can help them use the debugger for OS X.

StevenKersting commented 8 years ago

I'll be happy to try and help... if the crash is causing some of the issues I saw it might be more important than I originally thought.

mischnic commented 8 years ago

XCode log, after closing the app using the red x

2016-02-26 14:05:43.684 MIDI2LR[14057:129382] App Transport Security has blocked a cleartext HTTP (http://) resource load since it is insecure. Temporary exceptions can be configured via your app's Info.plist file. 2016-02-26 14:05:55.579 MIDI2LR[14057:129267] * Assertion failure in +[NSEvent startPeriodicEventsAfterDelay:withPeriod:], /Library/Caches/com.apple.xbs/Sources/AppKit/AppKit-1404.34/AppKit.subproj/NSEvent.m:3695 2016-02-26 14:05:55.579 MIDI2LR[14057:129267] An uncaught exception was raised 2016-02-26 14:05:55.579 MIDI2LR[14057:129267] Periodic events are already being generated 2016-02-26 14:05:55.580 MIDI2LR[14057:129267](0 CoreFoundation 0x9b58df79 __raiseError + 201 1 libobjc.A.dylib 0x919d4fd1 objc_exception_throw + 276 2 CoreFoundation 0x9b58ddfa +[NSException raise:format:arguments:] + 138 3 Foundation 0x92e5632b -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 118 4 AppKit 0x924ff1bf +[NSEvent startPeriodicEventsAfterDelay:withPeriod:] + 149 5 MIDI2LR 0x00058c62 _ZN4juce14MessageManager16stopDispatchLoopEv + 242 6 MIDI2LR 0x000103c6 _ZN18MIDI2LRApplication8shutdownEv + 534 7 MIDI2LR 0x00058661 _ZN4juce19JUCEApplicationBase11shutdownAppEv + 417 8 MIDI2LR 0x0000b88f main + 527 9 libdyld.dylib 0x9bb776ad start + 1) 2016-02-26 14:05:55.580 MIDI2LR[14057:129267] * Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Periodic events are already being generated' *\ Call stack at first throw: ( 0 CoreFoundation 0x9b58df79 __raiseError + 201 1 libobjc.A.dylib 0x919d4fd1 objc_exception_throw + 276 2 CoreFoundation 0x9b58ddfa +[NSException raise:format:arguments:] + 138 3 Foundation 0x92e5632b -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 118 4 AppKit 0x924ff1bf +[NSEvent startPeriodicEventsAfterDelay:withPeriod:] + 149 5 MIDI2LR 0x00058c62 _ZN4juce14MessageManager16stopDispatchLoopEv + 242 6 MIDI2LR 0x000103c6 _ZN18MIDI2LRApplication8shutdownEv + 534 7 MIDI2LR 0x00058661 _ZN4juce19JUCEApplicationBase11shutdownAppEv + 417 8 MIDI2LR 0x0000b88f main + 527 9 libdyld.dylib 0x9bb776ad start + 1 )

bildschirmfoto 2016-02-26 um 14 07 06

rsjaffe commented 8 years ago

Thanks--that gives some clues. I didn't write that part of the application so I'm going to have to take some time to get familiar with it and with how the Juce framework functions. There are a number of timers, observers, and loops going on, and, if I read the stack correctly, there may be a problem with how the message loop terminates that causes problems with an OS X timer.

rsjaffe commented 8 years ago

Does it still crash with 0.9.10? I think I fixed some of the issues.

mischnic commented 8 years ago

After removing the non existent file RecentFilesMenuTemplate.nib from the project, these are the error-s/lines:

m_commandMap->toXMLDocument(defaultProfile); in MIDI2LRApplication::shudown()

for (auto mapEntry : _messageMap) in CommandMap::toXMLDocument

bildschirmfoto 2016-04-04 um 18 51 53 bildschirmfoto 2016-04-04 um 18 52 16

rsjaffe commented 8 years ago

Excellent information. Looks like _messageMap is deleted before defaults written out during shutdown. I'll try to track this down.

rsjaffe commented 8 years ago

Could you make the following edit to Main.cpp and see if it still crashes?

change the order of shutdown in void shutdown() by changing from:

 void shutdown() override
    {
        // Save the current profile as default.xml
        auto defaultProfile = File::getSpecialLocation(File::currentExecutableFile).getSiblingFile("default.xml");
        m_commandMap->toXMLDocument(defaultProfile);
        m_lr_IPC_OUT.reset();
        m_lr_IPC_IN.reset();
        //below resets added
        m_commandMap.reset();
        m_profileManager.reset();
        m_settingsManager.reset();
        m_midiProcessor.reset();
        m_midiSender.reset();
        mainWindow = nullptr; // (deletes our window)
        quit();
    }

to

 void shutdown() override
    {
        // Save the current profile as default.xml
        auto defaultProfile = File::getSpecialLocation(File::currentExecutableFile).getSiblingFile("default.xml");
        m_commandMap->toXMLDocument(defaultProfile);
        m_lr_IPC_OUT.reset();
        m_lr_IPC_IN.reset();
        //below resets added
        m_profileManager.reset();
        m_settingsManager.reset();
        m_midiProcessor.reset();
        m_midiSender.reset();
        m_commandMap.reset();
        mainWindow = nullptr; // (deletes our window)
        quit();
    }
mischnic commented 8 years ago
 void shutdown() override
    {
        // Save the current profile as default.xml
        auto defaultProfile = File::getSpecialLocation(File::currentExecutableFile).getSiblingFile("default.xml");
---->   m_commandMap->toXMLDocument(defaultProfile); 
        m_lr_IPC_OUT.reset();
        m_lr_IPC_IN.reset();
        //below resets added
  ~     m_profileManager.reset();
  ~     m_settingsManager.reset();
  ~     m_midiProcessor.reset();
  ~     m_midiSender.reset();
  ~     m_commandMap.reset();
        mainWindow = nullptr; // (deletes our window)
        quit();
    }

It crashes on the second line, so changing anything further down won't change anything.