← Back to team overview

kicad-developers team mailing list archive

Re: GAL canvas deadlock

 

Hi Seth,

That doesn’t work because the lock/unlock calls are in separate routines (BeginDrawing() and EndDrawing()).

That’s fundamentally unsafe, so I’m adding lockContext() and unlockContext() protected methods to the GAL API and a GAL_CONTEXT_LOCKER friend class that can be created on the stack.  BeginDrawing() will ASSERT that the context has indeed been locked.

Sound reasonable?

Cheers,
Jeff.

> On 12 Oct 2018, at 01:12, Seth Hillbrand <seth@xxxxxxxxxxxxx> wrote:
> 
> Hmm... That makes sense.  In the calls, we only unlock for std::runtime_error.  Since you can trigger this, could you add a default catch with an unlock call?  (opengl_gal.cpp:357 and 2009)
> 
> -Seth
> 
> Am Do., 11. Okt. 2018 um 16:51 Uhr schrieb Jeff Young <jeff@xxxxxxxxx <mailto:jeff@xxxxxxxxx>>:
> Hi Seth,
> 
> I agree that it would be better to get to the bottom of this.  I just wasn’t having any luck.
> 
> The context locking issue has to do with trying to go to fallback canvas while in the middle of rendering.  But we do that (in both Tom’s and my case) as a result of catching an error from OpenGL.
> 
> So the locking issue isn't specific to Mac, just the error that leads to it (in my case; probably not in Tom’s case).
> 
> Cheers,
> Jeff.
> 
> 
>> On 11 Oct 2018, at 20:59, Seth Hillbrand <seth@xxxxxxxxxxxxx <mailto:seth@xxxxxxxxxxxxx>> wrote:
>> 
>> 
>> 
>> Am Do., 11. Okt. 2018 um 12:20 Uhr schrieb Jeff Young <jeff@xxxxxxxxx <mailto:jeff@xxxxxxxxx>>:
>> I pushed a change that keeps the error from happening for me.  It now makes sure it updates the GAL settings in the active window first and then any others.
>> 
>> @Seth, if you want to look into it on your Mac, the change is in KIWAY::CommonSettingsChanged().
>> 
>> Hi Jeff-
>> 
>> I have concerns about this approach.  Since it seems to work, my guess is that there is a locking time difference on the Mac that leads to this issue.  I think that we need to fix the context locking mechanism as there is clearly a way to lock a context and not unlock it after.  
>> 
>> Do you see the locking issue when running a debug build or only in Release?
>> 
>> -Seth
> 


Follow ups

References