vlc | branch: master | Josh Watzman <[email protected]> | Sun Jan 6 12:28:57 2013 -0800| [a8ff52782d21b6425d8c26c96a2269939ac6ce18] | committer: Felix Paul Kühne
macosx: call input_changed in extensions This is obnoxiously complicated. If anyone cares about playing_changed or meta_changed, something similar will probably have to be done. This is a pretty bizarre two-step system to inform the extension manager that the input has changed, but it's necessary to avoid a series of possible deadlocks and other issues. Here are other possible approaches that don't work: - Just call into the extension manager in -PlaylistItemChanged on the main thread. This can pretty easily cause a deadlock if we call -PlaylistItemChanged twice in quick succession. The first call will poke the condvar the extension is waiting on, causing the extension thread to wake up and run extension code; many parts of it -- including the dialog code -- must be run on the main thread. The extension thread goes back to sleep while blocking on the main thread to become available, while holding the extension lock. Meanwhile the main thread goes into the second call of -PlaylistItemChanged, attempts to lock the extension, and that's a deadlock. - Restructure the dialog manager to never block on the main thread while holding the extension lock. This should work, but as it turns out doesn't because the main thread will attempt to lock the same lock twice. What happens is that -performSelectorOnMainThread works by injecting an event into the main event loop of the main thread. For some unknown reason, as part of its processing, when creating an NSAttributedString with HTML, it runs the main event loop, which means we can end up executing one -performSelectoOnMainThread as part of another. Since the dialog manager uses attributed strings with HTML (since dialogs are HTML), we deadlock here too. This seems strictly like a flaw in NSAttributedString and/or in -performSelectorOnMainThread and is documented elsewhere: http://mrrsoftware.com/blog/tag/nsattributedstring/ https://www.bluestatic.org/blog/2010/05/31/nsattributedstring-spins-a-nested-run-loop/ - Change around this bit of code to not force it to run on the main thread. This would probably work, but, as a newcomer to VLC, I don't quite know the implications of doing this, particularly since a lot of code here seems to serailize on the main thread as a way of thread safety; it would likely require some somewhat intricate restructuring and adding of locks. - Let the extension manager deal with listening for events the same way that we do here. That would work, but would require duplicating a nontrivial amount of code from here to deal with tracking the current input. - So, instead, we just serialized all calls to -PlaylistItemChanged (so we make sure to process them in order, with no one trampling p_input_changed), do most of the work on the main thread as before, and then actually inform the extension manager out here where we don't block the main thread. It seems likely that there are other pre-existing deadlock possibilities here -- the main thread can't lock an extension! -- but it at least tends to work in my testing. Signed-off-by: Felix Paul Kühne <[email protected]> > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=a8ff52782d21b6425d8c26c96a2269939ac6ce18 --- modules/gui/macosx/intf.h | 2 +- modules/gui/macosx/intf.m | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/modules/gui/macosx/intf.h b/modules/gui/macosx/intf.h index cf9edde..5fa2490 100644 --- a/modules/gui/macosx/intf.h +++ b/modules/gui/macosx/intf.h @@ -83,7 +83,7 @@ struct intf_sys_t @interface VLCMain : NSObject <NSWindowDelegate, NSApplicationDelegate> { intf_thread_t *p_intf; /* The main intf object */ - input_thread_t *p_current_input; + input_thread_t *p_current_input, *p_input_changed; id o_mainmenu; /* VLCMainMenu */ id o_prefs; /* VLCPrefs */ id o_sprefs; /* VLCSimplePrefs */ diff --git a/modules/gui/macosx/intf.m b/modules/gui/macosx/intf.m index 6ed1985..3aa9f66 100644 --- a/modules/gui/macosx/intf.m +++ b/modules/gui/macosx/intf.m @@ -62,6 +62,7 @@ #import "CoreInteraction.h" #import "TrackSynchronization.h" #import "VLCVoutWindowController.h" +#import "ExtensionsManager.h" #import "VideoEffects.h" #import "AudioEffects.h" @@ -252,6 +253,7 @@ void WindowClose(vout_window_t *p_wnd) * Run: main loop *****************************************************************************/ static NSLock * o_appLock = nil; // controls access to f_appExit +static NSLock * o_plItemChangedLock = nil; static void Run(intf_thread_t *p_intf) { @@ -259,12 +261,14 @@ static void Run(intf_thread_t *p_intf) [VLCApplication sharedApplication]; o_appLock = [[NSLock alloc] init]; + o_plItemChangedLock = [[NSLock alloc] init]; [[VLCMain sharedInstance] setIntf: p_intf]; [NSBundle loadNibNamed: @"MainMenu" owner: NSApp]; [NSApp run]; [[VLCMain sharedInstance] applicationWillTerminate:nil]; + [o_plItemChangedLock release]; [o_appLock release]; [o_pool release]; @@ -381,7 +385,14 @@ static int PLItemChanged(vlc_object_t *p_this, const char *psz_var, vlc_value_t oldval, vlc_value_t new_val, void *param) { NSAutoreleasePool * o_pool = [[NSAutoreleasePool alloc] init]; - [[VLCMain sharedInstance] performSelectorOnMainThread:@selector(PlaylistItemChanged) withObject:nil waitUntilDone:NO]; + + /* Due to constraints within NSAttributedString's main loop runtime handling + * and other issues, we need to wait for -PlaylistItemChanged to finish and + * then -informInputChanged on this non-main thread. */ + [o_plItemChangedLock lock]; + [[VLCMain sharedInstance] performSelectorOnMainThread:@selector(PlaylistItemChanged) withObject:nil waitUntilDone:YES]; + [[VLCMain sharedInstance] informInputChanged]; + [o_plItemChangedLock unlock]; [o_pool release]; return VLC_SUCCESS; @@ -582,7 +593,7 @@ static VLCMain *_o_sharedMainInstance = nil; _o_sharedMainInstance = [super init]; p_intf = NULL; - p_current_input = NULL; + p_current_input = p_input_changed = NULL; o_msg_lock = [[NSLock alloc] init]; o_msg_arr = [[NSMutableArray arrayWithCapacity: 600] retain]; @@ -1279,7 +1290,7 @@ static VLCMain *_o_sharedMainInstance = nil; { if (p_current_input && (p_current_input->b_dead || !vlc_object_alive(p_current_input))) { var_DelCallback(p_current_input, "intf-event", InputEvent, [VLCMain sharedInstance]); - vlc_object_release(p_current_input); + p_input_changed = p_current_input; p_current_input = NULL; [o_mainmenu setRateControlsEnabled: NO]; @@ -1293,6 +1304,7 @@ static VLCMain *_o_sharedMainInstance = nil; [o_mainmenu setRateControlsEnabled: YES]; if ([self activeVideoPlayback] && [[o_mainwindow videoView] isHidden]) [o_mainwindow performSelectorOnMainThread:@selector(togglePlaylist:) withObject: nil waitUntilDone:NO]; + p_input_changed = vlc_object_hold(p_current_input); } } @@ -1302,6 +1314,15 @@ static VLCMain *_o_sharedMainInstance = nil; [self updateMainMenu]; } +- (void)informInputChanged +{ + if (p_input_changed) { + [[ExtensionsManager getInstance:p_intf] inputChanged:p_input_changed]; + vlc_object_release(p_input_changed); + p_input_changed = NULL; + } +} + - (void)updateMainMenu { [o_mainmenu setupMenus]; _______________________________________________ vlc-commits mailing list [email protected] http://mailman.videolan.org/listinfo/vlc-commits
