<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 28, 2015 at 12:47 PM, Chris Muller <span dir="ltr"><<a href="mailto:asqueaker@gmail.com" target="_blank">asqueaker@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Wait, the newer one has a non-local return in it, but<br>
Mutex>>#critical: has an ensure: in it anyway, so maybe I don't see<br>
the problem..?<br></blockquote><div><br></div><div>If one hits ctrl-period when the system is in the critical section then the debugger can't open because it interrupts the critical section, preventing the ensure block from running, attempts to access e.g. scroll bar preferences when it tries to open, and the system deadlocks. So preferences either need to be *not* protected by a critical section, or the Debugger needs not to access preferences.</div><div><br></div><div>IMO, we should try and write preferences so that they don't require a lock. Writing them as a lock-free data structure would be a really good idea. First that critical section is slow and clunky. Second, I presume it is there only for the rare case of a write to preferences, not to protect reads.</div><div><br></div><div>IMO, a simple implementation which copied and replaced the entire preferences dictionary on write would be sufficient. Sure there's a danger that some client would get a stale value if it read concurrently while there was a write, but then so what? A preference is a preference, not a hard-and-fast value, and code should work accessing a preference no matter its value, so momentarily getting a stale value shouldn't matter. So the implementation could be as simple as</div><div><br></div><div><div>addPreference: aName categories: categoryList default: aValue balloonHelp: helpString projectLocal: localBoolean changeInformee: informeeSymbol changeSelector: aChangeSelector type: aType</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>"Add or replace a preference as indicated. Reuses the preexisting Preference object for this symbol, if there is one, so that UI artifacts that interact with it will remain valid."</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>| aPreference aPrefSymbol |</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>aPrefSymbol := aName asSymbol.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>aPreference := DictionaryOfPreferences</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>at: aPrefSymbol</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>ifAbsent:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>[| newPreference |</div><div><span class="Apple-tab-span" style="white-space:pre"> </span> newPreference := aPreference </div><div><span class="Apple-tab-span" style="white-space:pre"> </span>name:aPrefSymbol</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>defaultValue:aValue</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>helpString:helpString</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>localToProject:localBoolean</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>categoryList:categoryList</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>changeInformee:informeeSymbol</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>changeSelector:aChangeSelector</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>type: aType.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span> AccessLock critical:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>[| newDict |</div><div><span class="Apple-tab-span" style="white-space:pre"> </span> newDict := DictionaryOfPreferences copy.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span> newDict at: aPrefSymbol put: newPreference].</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>self compileAccessMethodForPreference:aPreference.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span> newPreference]</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On Tue, Apr 28, 2015 at 2:43 PM, Chris Muller <<a href="mailto:asqueaker@gmail.com">asqueaker@gmail.com</a>> wrote:<br>
>> The above change restores the old behavior of locking up the image, so it<br>
>> should be reverted. An additional comment explaininng why aBlock must not be<br>
>> evaluated inside the argument of #accessDictionaryOfPreferencesIn: would be<br>
>> helpful.<br>
><br>
> Ahh, because aBlock might have a non-local return in it, leaving the<br>
> Mutex unsignaled (and critical unenterable), is that right?<br>
><br>
> Took me a minute to see that problem.<br>
><br>
> Okay, I'll revert that method if no one else does by my next commit..<br>
><br>
>> It would be even better to finally get rid of DictionaryOfPreferences.<br>
>><br>
>><br>
>> Levente<br>
>><br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">best,<div>Eliot</div></div>
</div></div>