Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006520 [Squeak] Multilingual crash always 06-02-07 18:46 11-20-08 20:12
Reporter RalphJohnson View Status public  
Assigned To
Priority high Resolution fixed  
Status closed   Product Version 3.10
Summary 0006520: recursive fonts cause crash when running tests
Description When I try to run all tests in 3.10alpha.7105, the image often crashes. Sometimes I can run the tests once, but the second time I run them then the system crashes. The problem is that a TTCFont has itself as its "fallbackFont", and this causes infinite recursion in methods like #ascent. I fixed #ascent but something else broke.

Diego Deck worked on this. He said "The problem (the one I found) is fired from
LocaleTest>>testLocaleChanged. Probably the problem is that the first
round of test-runs change the default fonts, and the #localeChanged
propagation fires TTCFont>>setupDefaultFallbackFont, that changes the
receiver's fallbackFont to the default font."

Diego gave me the a .cs file with two changed methods. This solves the problem for me.
Now I can run tests over and over without any problems, though several tests fail that did not fail before.
Additional Information
Attached Files  FontFixes-dgd.1.cs [^] (774 bytes) 06-02-07 18:46
 Font-fix-rej1-M6520.cs [^] (1,314 bytes) 09-29-07 17:54

- Relationships

SYSTEM WARNING: Creating default object from empty value

SYSTEM WARNING: Creating default object from empty value

related to 0006524closed  TTFonts can't load unicode character maps 
related to 0006664assigned andreas TTCFont>ascent can be infinitely recursive. 
child of 0006570assigned tim A Mother for font and font test problems 

- Notes
(0011223 - 1625 - 1667 - 1667 - 1667 - 1667 - 1667)
RalphJohnson
09-29-07 17:53

I fixed another recursive font problem, and now the font tests in beta.7143 run without crashing.

As Jerome said in 000664, not only are StrikeFone and TTCFont recursive (because they depend on a fallbackFont) but also FixedFaceFont is recursive, because it has a baseFont. It turns out that StrikeFonts often have a nil fallbackFont, but a FixedFaceFont and a TTCFont always depend on another font. FixedFaceFont is written so that it is ovbvious that its baseFont is never nil, but a lot of the code in TTCFont checks for a nil fallbackFont, so it looks at first glance like its fallbadkFont is option. However, the fallbackFont method creates a default fallback font if it doesn't have one, so in fact each TTCFont depends on another font.

One solution might be to change TTCFont>>fallbackFont to not create a default font. However, I took a different approach. I changed it so that the default font is always a StrikeFont. I changed both StrikeFont and FixedFaceFont so that their default font is StrikeFont. Perhaps I should have changed TTCFont as well, but it continues to use the default font (which is often a TTCFont) as its default fallback font. You can make a StrikeFont or a FixedFaceFont depend on a TTCFont if you want, but it will no longer happen by default when you switch the default font to be a TTCFont.

The font classes could use a rewrite. I agree with Andreas that TTCFont and StrikeFont shouldn't have a fallbadkFont but the system should instead use the Composte pattern to specify that characters not defined by one font should be defined by another. However, i did not do that.
 
(0011225 - 1053 - 1137 - 1137 - 1137 - 1137 - 1137)
wiz
09-30-07 03:25

Hi Ralph,

I think your fixes patch the symtoms but are not the correct fixes.

As long as a TTCFont exists in the system it should probably be allowed as a default font. The repair should be to the TTCFont message code so that if follows the assumptions and rules that the same messages in other polymorphic classes follow.

The messages that TTCFont uses should be repaired to not be infinitely recursive. (i.e. test for exit case first before using a potentially recursive method.) And the rules that it (and all the other fonts) need to follow need to be stated explicitly somewhere. So coders can find them.

What you have done here is a work-around which hides the problem rather than fixing it.

I'll let others decide if that is ok. And I acknowledge that under time constrains a patch may be all that can be achieved.

Still I hope someone would give a run at the true fix. Or at least add pieces of the fix to a report until there is enough to attempt a non-limiting solution.

Yours in curiosity and service, --Jerome Peace
 
(0011226 - 1106 - 1154 - 1154 - 1154 - 1154 - 1154)
RalphJohnson
09-30-07 14:58

On the one hand, I agree that I patched the system, and it probably needs a rewrite. On the other hand, I think I understand the system pretty well and that my patch fits in with the design.

TTCFonts *are* allowed as default fonts. That is why the default font is not the default backup font for TTCFont.

TTCFont is different from StrikeFont, even though they both have backup fonts. The TTCFont backup font is mandatory, while the StrikeFont backup font is optional. Neither classes have an explicit base case. The base case is a StrikeFont or a TTCFont with no backup font. Since a TTCFont must have a backup front, it cannot be the base case. Only a StrikeFont can be the base case.

TTCFont and StrikeFont are both examples of Chain of Responsibility. The end of the chain will always be a StrikeFont with no backup font.

Although a lot of the code in TTCFont implies that backupFont is optional, in fact methods like #glyphInfoOf:Into: assume that it is required. So, changing TTCFont to make the backup font optional would require more than just changing the #backupFont method.
 
(0011227 - 2023 - 2191 - 2191 - 2191 - 2191 - 2191)
wiz
10-01-07 05:09

Hi Ralph,

Our objectives are aligned but different. I need to satisfy my curiosity as to what the bug (or design flaw) really is, while you are in need of a way to proceed with 3dot10.

I learned something I needed to know from your reply.

I hadn't realized that backup fonts and fallbackfonts were not the same. There are probably some font code coders who out there in the same boat.

I am still approaching font stuff from the outside and making my deductions in the usual way. *

I am aware of two things.
The fonts have had many cooks.
Lots of bugs were introduced because the i18n coders had an incomplete picture of what the fonts were about and what invarients needed to be met. No one did testing until complaints were raised.

I question whether TTCfonts need a backup font. It seems to me the lesser constraint is that there need to be backup glyphs.

And if a TTCFont is in the system it could be a base font given it can do something when asked to display a glyph it knows nothing about. (And part of me says if an existing TTCFont doesn't know how to render a glyph how does the strikefont know better?)

The other curiosity I have is that of preserving font state thru a test. What base states have to be pushed before and popped after a test to insure global font state is back to normal. The tests don't say. My attempt with tiny and large fonts proved inadequate. The answer to this curiosity would permit better font testing.

[ot] but relevant. Toggling the tinyfont preference takes several seconds. This is a symptom of something not right in the font changing code. My current notion is that at the toggle point many strikefonts or font images are being generated. It's another indication of where to look for suspects.

Ah well,, its late and I already put in my squeak time today on a different project.

a suivre

Yours in curiosity and service, --Jerome Peace

*Usual method = Stumble around in the dark. In the dark, stumbles are very illuminating. -Jer
 
(0011232 - 638 - 682 - 682 - 682 - 682 - 682)
RalphJohnson
10-01-07 13:14

I'm sorry for being confusing. I should have said "fallback font" instead of "backup font". They mean the same thing to me.

The point I was trying to make (and I seemed not to have made properly) is that the problem is not any one object, it is the relationship between objects. Perhaps TTCFont shouldn't assume that it has a fallback font. But it does, and so it is important that the default backup font not be a TTCFont.

The problem with tests changing information about fonts is that some tests change the default font. They need to set it back. That is one of the many problem with Singletons; they make testing harder.
 
(0011233 - 1787 - 1937 - 1937 - 1937 - 1937 - 1937)
wiz
10-02-07 04:17

Hi Ralph,

Thanks for the response.

The distinction I think I was concerned with was between a "fallback font" and a "default fallback font". There seems to be a distintion needed there to prevent infinite recursion.

The relationship among fonts is very muddled. That's what confused and confounded me in my first attempt at a fix. The assumptions behind the broken code were so confusing that my first guess at a fix was far enough off to break things elsewhere.

Part of the problem is my notion that "code should not be so complicated or clever that it can not be understood at a glance". That's obviously an ideal but when I run into code that defies that principle too much I know there is serious trouble either already present or in the future.

The protocol for the font messages were not correctly understood by the modifiers of the code.

Part of the way out of this is that parrallel messages should be of similar complexity. #ascent violated this because it was usually just a straight forward accessor and TTCFont made it a lazily initialized method. There are other problems introduced by the coders e.g. the accuny strike font for large sizes renders spaces with unwanted glitches.

When you get careless coders modifing clever code lots of clever bugs result.

I haven't looked at the font stuff in a while. I will do so when I get a moment free. Then I'll see if I've learned anything useful since last time.

I think I did notice that there was a way to reinitialize the font defaults to their original values. It occured to me that that would be saner than letting the test just change them. Its not an ideal push and pop of the required state but its close enough for goverment work.

Yours in curiosity and service, --Jeorme Peace
 
(0011242 - 193 - 205 - 205 - 205 - 205 - 205)
edgardec
10-06-07 11:49

Reminder sent to: RalphJohnson

I very sorry to report fileIn the two change sets made the image go limbo when running the complete test.
This happens in Mac and also in Windows XP,
I look my old notes about this for clues.
 
(0011244 - 331 - 403 - 403 - 403 - 403 - 403)
wiz
10-06-07 19:04

Hi Edgar,

Can you be more specific about what broke.

When did the image go into limbo.

Is there any one test that can cause that problem?
(Lang Env tests and test that send #isFontAvailable are chief suspects)

Any debug logs or space-low logs come out of the problem?


Yours in curiosity and service, -Jerome Peace
 
(0012169 - 473 - 509 - 509 - 608 - 608 - 608)
wiz
05-26-08 18:29

Though the issue has not been marked both of these changes have made it in to 3.10 (and 3.10.1).

I am convinced that these patches are very poor work arounds for the real problem which it that TTCFonts needs to be writen to avoid recursion.
In particular ascent needs to return to being a simple accessor.
And setting fallbackfont needs to update the value of ascent and maybe a few other things that rely on it.

Then there is no recursion. See the comment in 0006664
 
(0012787 - 65 - 65 - 65 - 65 - 65 - 65)
KenCausey
11-20-08 20:12

Harvested in updates 7122 and 7148 and released with Squeak 3.10.
 

- Issue History
Date Modified Username Field Change
06-02-07 18:46 RalphJohnson New Issue
06-02-07 18:46 RalphJohnson File Added: FontFixes-dgd.1.cs
07-19-07 23:51 wiz Relationship added related to 0006524
09-12-07 00:11 wiz Relationship added child of 0006570
09-12-07 00:15 wiz Relationship added related to 0006664
09-29-07 17:53 RalphJohnson Note Added: 0011223
09-29-07 17:54 RalphJohnson File Added: Font-fix-rej1-M6520.cs
09-30-07 03:25 wiz Note Added: 0011225
09-30-07 14:58 RalphJohnson Note Added: 0011226
10-01-07 05:09 wiz Note Added: 0011227
10-01-07 13:14 RalphJohnson Note Added: 0011232
10-02-07 04:17 wiz Note Added: 0011233
10-06-07 11:49 edgardec Note Added: 0011242
10-06-07 19:04 wiz Note Added: 0011244
05-26-08 18:29 wiz Note Added: 0012169
05-26-08 18:29 wiz Status new => feedback
11-20-08 20:12 KenCausey Status feedback => closed
11-20-08 20:12 KenCausey Note Added: 0012787
11-20-08 20:12 KenCausey Resolution open => fixed
11-20-08 20:12 KenCausey Fixed in Version  => 3.10


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