|Anonymous | Login||05-11-2021 13:52 UTC|
|Main | My View | View Issues | Change Log | Docs|
|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|
|Summary||0002701: DependentsArray might potentially be misused|
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.
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.
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
(0010072 - 228 - 240 - 240 - 240 - 240 - 240)
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)
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)
Installer mantis bug:2701 fix: 'DependentsPatch.4.cs'.
Installer mantis bug:2701 fix: 'DependentsTest.2.cs'.
(0013383 - 230 - 260 - 570 - 570 - 570 - 570)
Note: The fix is different from changeset provided here.
I preferred to change hierarchy of DependentsArray
|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.