On Tue, 16 Feb 2010, Juan Vuletich wrote:

> Levente Uzonyi wrote:
>> On Tue, 16 Feb 2010, Juan Vuletich wrote:
>> 
>>> commits at source.squeak.org wrote:
>>>> Levente Uzonyi uploaded a new version of System to project The Trunk:
>>>> http://source.squeak.org/trunk/System-ul.253.mcz
>>>> 
>>>> ==================== Summary ====================
>>>> 
>>>> Name: System-ul.253
>>>> Author: ul
>>>> Time: 14 February 2010, 9:12:11.268 am
>>>> UUID: 7aaed65b-d0e3-0040-8929-0eb0bf8ecf64
>>>> Ancestors: System-ar.252
>>>> 
>>>> - fix (again): Timer may be already nil in the #ensure: block in 
>>>> MessageTally >> #spyEvery:on: and MessageTally >> #spyAllEvery:on:
>>>> 
>>> 
>>> Hi Levente,
>>> 
>>> How could that happen? Can you post a script to reproduce it?
>> 
>> Well, the old way to reproduce this is fixed (press alt/cmd+. while 
>> profiling) and that's great. But there's another one, it happens when 
>> you're tallying something which tallies something else. I know this 
>> shouldn't happen normally, but we have some tests which use MessageTally 
>> (for no reason). If you press the "Run Profiled" button in TestRunner, 
>> these tests will set Timer's value to nil and at the end, you get a 
>> debugger.
>> 
>> 
>> Levente
>
> Thanks, I see. Anyway, nested tallies are not working at all (and never 
> worked). Your fix might give the impression that nested tallies are ok, when 
> they are not. Perhaps it would be best to just raise an error.

I agree that it's not the best solution and it shadows an issue. We could 
just fix the tests and revert the code. But I think that changing the 
value of a class variable from instance methods is a sign of bad design. 
Why is MessageTally using the class variable Timer instead of an instance 
variable?


Levente

P.S.:
I added a new instance variable timer to MessageTally, replaced 
all occurence of Timer to timer, removed the #ifNotNil: checks and 
successfully profiled the tests. Is anything wrong with this approach?

>
> Cheers,
> Juan Vuletich
>
>