After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 695913 - test-pangocairo-threads fails on Win32
test-pangocairo-threads fails on Win32
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: win32
1.33.x
Other Windows
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
pango-maint
Depends on:
Blocks:
 
 
Reported: 2013-03-15 14:24 UTC by Fan, Chun-wei
Modified: 2014-04-10 03:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
image that indicates failure (793 bytes, image/png)
2013-03-15 14:28 UTC, Fan, Chun-wei
  Details
Improve the thread-safety situation in pangowin32-fontmap.c (3.35 KB, patch)
2014-02-20 02:28 UTC, Fan, Chun-wei
rejected Details | Review
Improve the thread-safety situation in pangowin32-fontmap.c (take ii) (4.75 KB, patch)
2014-02-21 03:45 UTC, Fan, Chun-wei
none Details | Review
Fix the build of the pushed patch (2.44 KB, patch)
2014-04-10 03:45 UTC, Fan, Chun-wei
none Details | Review

Description Fan, Chun-wei 2013-03-15 14:24:08 UTC
Hi,

In the process of making the Win32 backend threadsafe, I was building and running the test-pangocairo-threads test program, which failed with the following output:

-A whole bunch of messages, as the following line:
(test-pangocairo-threads.exe:1312): Pango-WARNING **: All font fallbacks failed!
!!! (the number "1312" is different with every run)
-A message that says "image for thread 0 different from reference image."

This means the issue mentioned in https://bugzilla.gnome.org/show_bug.cgi?id=377539#c73 is yet to be investigated and fixed :|

With blessings, thank you!
Comment 1 Fan, Chun-wei 2013-03-15 14:28:11 UTC
Created attachment 238981 [details]
image that indicates failure

This is the image I get as output as the test failed.
Comment 2 Behdad Esfahbod 2013-03-15 21:34:26 UTC
I also checked under wine and get the same results.  Don't know enough about the win32 backend to investigate right now.
Comment 3 Michael 2013-12-07 19:55:31 UTC
Got the same results with Pango 1.36.1
Comment 4 Fan, Chun-wei 2013-12-20 01:15:53 UTC
Hi,

It does seem more and more to me that Cairo needs to be dealt with on Windows in terms of thread safety (perhaps along with PangoWin32).  I tried to update PangoWin32 for thread safety, but still failed by getting the same problem-looking back to it that FontMaps were used as a GPrivate in PangoCairo.

Any further insights or observations would be great.

With blessings, thank you!
Comment 5 Behdad Esfahbod 2013-12-21 01:18:23 UTC
I have no suggestions.
Comment 6 Fan, Chun-wei 2013-12-24 07:43:20 UTC
Hi,

Upon some searching, it does seem that this is both a PangoWin32 issue and a Cairo-Win32 font issue[1], adapting the suggested patch from the given link [1] does make the test program fare better on Windows, at least I was able to run 30 consecutive runs of the test without crashing (previously the crashes reduced, but still occur intermittently), although the test still does fail.

It's at least getting better, still striving forward for this.

I will open a bug at Cairo's bugzilla for this.

Well, Merry Christmas, and with blessings!

[1]: http://lists.cairographics.org/archives/cairo/2010-July/020297.html
Comment 7 Fan, Chun-wei 2013-12-24 09:36:32 UTC
Hi,

I have filed a bug against Cairo in https://bugs.freedesktop.org/show_bug.cgi?id=73012 for this issue.

With blessings, thank you, and Merry Christmas!
Comment 8 Fan, Chun-wei 2014-02-20 02:28:00 UTC
Created attachment 269749 [details] [review]
Improve the thread-safety situation in pangowin32-fontmap.c

Hi,

This is an attempt to improve the thread-safety situation on PangoWin32 by reading the aliases only during initialization of PangoWin32, and ensuring that it is only done once.  Any suggestions on this patch is welcome-this does not yet make Pango thread-safe on Windows, but I think it at least improves the situation as the test-pangocairo-threads does not crash anymore, if used with the fix that I proposed for Cairo (in the link suggested by the patch's commit message).

With blessings, thank you!
Comment 9 Behdad Esfahbod 2014-02-20 20:17:41 UTC
Comment on attachment 269749 [details] [review]
Improve the thread-safety situation in pangowin32-fontmap.c


>+  if (g_once_init_enter (&ht_loaded))
>+    {
>+      if (pango_aliases_ht == NULL)
>+        load_aliases ();
>+      g_once_init_leave (&ht_loaded, 1);
>+    }

You only need a very short change, no mutex, etc.  Just make load_aliases() return the hash-table instead of setting pango_aliases_ht directly, and then in lookup_aliases(), add g_once_init_* instead of the current:

 
  if (pango_aliases_ht == NULL) 
    load_aliases ();
Comment 10 Fan, Chun-wei 2014-02-21 03:45:06 UTC
Created attachment 269871 [details] [review]
Improve the thread-safety situation in pangowin32-fontmap.c (take ii)

Hello Behdad,

I saw your suggestions, so is this better for you?

Thanks though, with blessings.
Comment 11 Behdad Esfahbod 2014-04-09 23:58:54 UTC
I pushed out a modified patch.  Please test.
Comment 12 Fan, Chun-wei 2014-04-10 03:02:35 UTC
Hi Behdad,

Just wondering, did I miss something?  Just did a 'git pull origin master' on git.gnome.org/pango and the last patch was the patch I did for building introspection with Windows builds...

Thanks very much though-just also wondering whether this patch from you is on Cairo?

With blessings, thank you!
Comment 13 Behdad Esfahbod 2014-04-10 03:17:51 UTC
Oops.  Pushed.

Not sure what you meant re cairo.
Comment 14 Fan, Chun-wei 2014-04-10 03:45:24 UTC
Created attachment 273952 [details] [review]
Fix the build of the pushed patch

Hi Behdad,

It seems to do the job, though PangoWin32 is not yet thread-safe (so I think it goes down to Cairo further, or so).  I did, however, needed to update the patch as it had problems on pointer level indirection, that caused warnings/errors.

Thanks very much, with blessings.
Comment 15 Behdad Esfahbod 2014-04-10 03:51:16 UTC
Fixed differently.