Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0002701 [Squeak] Kernel minor always 02-09-06 22:51 04-18-10 21:59
Reporter nicolas cellier View Status public  
Assigned To andreas
Priority normal Resolution fixed  
Status closed   Product Version 3.8
Summary 0002701: DependentsArray might potentially be misused
Description Problem identified is the risk of having a window closed without saving the changes among others.

I can exhibit a failure test case built on a silly example (see attachment uploaded),
but i do not know if the potential problem will express in a real case,
because this bug was found from bottom up code analysis.

See subject (DependentsArray size how does it work ?) on squeak-dev for details.

I also attached an incomplete unsatisfying patch not robust to future code evolutions. See additional info and attachments.
Additional Information evaluating
  dep := self dependents.
  1 to: dep size do: [:i | (dep at: i) doSomething]
might not be correct.

if some weak dependent has been reclaimed by garbage collector (see definition of class DependentsArray), then it is replaced by nil in dependents collection.
The above loop might end prematurely because #size will count only the non nil.
The loop might be evaluated with some nilled elements and not evaluated with some non nil dependents.

It happens that such (1 to:self size do:) construction is called indirectly in 3.8 image either via message #do:without: and also with #includes:.

Also, a (self dependents first) is called in TestRunner that could incorrectly answer nil. I presume this cannot be the case while the window is opened.

Check calls on #dependents.

Attached patch is non satisfying regarding lack of robustness to future code evolution: i just redefined afew collection messages in DependentArray so as not to use super #to:do: construct.

Almost each and every SequenceableCollection messages use this to:do: construct... If one ever send an unprotected message to its dependents, some other bug will potentially appear.

Maybe DependentsArray could also redefine #at:, but i do not know if it might break some methods and it would transform loop budget from n to n*n...

More robust solution should be searched for.

I suggest this bug remain open, or stored somewhere until better patch found.
Attached Files  DependentsPatch.4.cs [^] (3,152 bytes) 02-09-06 22:51
 DependentsTest.1.cs [^] (1,493 bytes) 02-09-06 22:53
 DependentsTest.2.cs [^] (1,921 bytes) 02-21-07 23:08

- Relationships

- Notes
(0010072 - 228 - 240 - 240 - 240 - 240 - 240)
nicolas cellier
02-21-07 23:22

A new test is added in DependentsTest.2.cs showing how trivially broken is the current implementation of DependentsArray.

I just add twice the same dependents, which should be prevented by the includes: test in #addDependent:
 
(0010073 - 408 - 477 - 477 - 477 - 477 - 477)
nicolas cellier
02-21-07 23:28

Ah, i found a good example of broken dependents consequence in 3.9 7067 image:

Open a regular browser,
Select a class (you should see class comment),
Save and quit the image,
Restart the image,
Select a method:
-> you see the place of class comment pane is not recycled...

Load DependentsPatch4.cs and replay above test
Ah it seems to work better...
Still not convinced about pending nasty bugs?
 
(0010094 - 145 - 199 - 199 - 199 - 199 - 199)
Keith_Hodges
02-22-07 06:21

"fix begin"
Installer mantis bug:2701 fix: 'DependentsPatch.4.cs'.
"fix test"
Installer mantis bug:2701 fix: 'DependentsTest.2.cs'.
"fix end"
 
(0013383 - 230 - 260 - 570 - 570 - 570 - 570)
nicolas cellier
11-14-09 21:29

Fixed in
http://source.squeak.org/trunk/Kernel-nice.299.mcz [^]
http://source.squeak.org/trunk/KernelTests-nice.108.mcz [^]

Note: The fix is different from changeset provided here.
I preferred to change hierarchy of DependentsArray
 

- Issue History
Date Modified Username Field Change
02-09-06 22:51 nicolas cellier New Issue
02-09-06 22:51 nicolas cellier File Added: DependentsPatch.4.cs
02-09-06 22:53 nicolas cellier File Added: DependentsTest.1.cs
07-07-06 19:16 rootbeer Issue Monitored: rootbeer
02-21-07 23:08 nicolas cellier File Added: DependentsTest.2.cs
02-21-07 23:22 nicolas cellier Note Added: 0010072
02-21-07 23:28 nicolas cellier Note Added: 0010073
02-22-07 06:18 Keith_Hodges Issue Monitored: Keith_Hodges
02-22-07 06:21 Keith_Hodges Note Added: 0010094
12-17-08 04:47 Keith_Hodges Status new => acknowledged
01-09-09 23:54 Keith_Hodges Status acknowledged => testing
01-10-09 03:39 Keith_Hodges Status testing => resolved
01-10-09 03:39 Keith_Hodges Fixed in Version  => 3.11
01-10-09 03:39 Keith_Hodges Resolution open => fixed
01-10-09 03:39 Keith_Hodges Assigned To  => Keith_Hodges
01-10-09 03:41 Keith_Hodges Status resolved => testing
10-03-09 19:34 Keith_Hodges Status testing => assigned
10-03-09 19:34 Keith_Hodges Assigned To Keith_Hodges => andreas
11-14-09 21:29 nicolas cellier Status assigned => resolved
11-14-09 21:29 nicolas cellier Fixed in Version 3.11 => trunk
11-14-09 21:29 nicolas cellier Note Added: 0013383
04-18-10 21:59 andreas Status resolved => closed


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
68 total queries executed.
39 unique queries executed.
Powered by Mantis Bugtracker