[vlc-devel] [PATCH] macosx: Prevent invalid access in viewWillMoveToWindow

[David Fuhrmann]

> Am 29.09.2016 um 19:26 schrieb Marvin Scholz <epirat07 at gmail.com>:
> 
> Previously it was not checked if vout display is null and
> trying to access sys->nsColorSpace would result in a crash.
> ---
> modules/video_output/macosx.m | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
> index 6c13eb6..f8f49a2 100644
> --- a/modules/video_output/macosx.m
> +++ b/modules/video_output/macosx.m
> @@ -839,7 +839,7 @@ - (void)viewWillMoveToWindow:(nullable NSWindow *)newWindow
>     [super viewWillMoveToWindow:newWindow];
> 
>     @try {
> -        if (newWindow != nil) {
> +        if (newWindow != nil && vd) {
>             @synchronized(newWindow) {
>                 [newWindow setColorSpace:vd->sys->nsColorSpace];
>             }

Hi,

Thanks for trying to fix the issue. But I think the fix is slightly wrong:
First, access to vd is protected by a mutex (@synchronized(self)), so this should be used instead of synchronized(newWindow)). Then, the check for "if (vd)“ needs to be moved inside the synchronized block.

In any case, I’m wondering why we do not have a retain count on vd here inside our view? Would it be fine to access a retained vd object inside the main thread, even if the vout was closed slightly before, in the vout thread?

This way, maybe we could avoid all those synchronized calls.

Best,
David