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 348212 - import single mbox crashes in pthreadGC2!pthread_mutex_lock
import single mbox crashes in pthreadGC2!pthread_mutex_lock
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Importers
2.6.x (obsolete)
Other Windows
: Normal critical
: ---
Assigned To: Tor Lillqvist
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2006-07-21 06:49 UTC by George V. Reilly
Modified: 2006-10-16 14:59 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
Suggested patch (3.45 KB, patch)
2006-07-27 16:14 UTC, Tor Lillqvist
committed Details | Review

Description George V. Reilly 2006-07-21 06:49:10 UTC
I'm running Evolution for Windows 2.6.2-5 from http://shellter.sourceforge.net/evolution/

I have a repeatable crash in the importers.

File > Import > Single file

Select NWCPP-Talk.mbox.txt, which can be obtained from http://www.georgevreilly.com/temp/NWCPP-Talk.mbox.txt
This mbox was generated with Thunderbird.

Click Forward, Inbox, Import. Boom!

Here's a callstack obtained in WinDbg:

0:000> kp 60
ChildEBP RetAddr  
WARNING: Stack unwind information not available. Following frames may be wrong.
0022ead8 005da2c9 pthreadGC2!pthread_mutex_lock+0x13
0022eb18 021d1885 libedataserver_1_2_7!e_thread_put+0x28
0022eb38 021d348e libevolution_mail_importers_0!mail_importer_import_mbox+0x74
0022eb68 003dd5cd libevolution_mail_importers_0!pine_importer_peek+0x4cb
0022eb88 00407556 libeutil_0!e_import_import+0x73
0022ebb8 62743935 image00400000+0x7556
0022ec18 62756e65 libgobject_2_0_0!g_closure_invoke+0x115
0022ed08 62757c0e libgobject_2_0_0!g_signal_has_handler_pending+0xf55
0022ef88 62757e86 libgobject_2_0_0!g_signal_emit_valist+0x7ee
0022efa8 6c859ff8 libgobject_2_0_0!g_signal_emit+0x26
0022efc8 6c858363 libgnomeui_2_0!gnome_druid_page_finish+0xab
0022eff8 62743935 libgnomeui_2_0!gnome_druid_get_type+0x16ee
0022f058 62756e65 libgobject_2_0_0!g_closure_invoke+0x115
0022f148 62757c0e libgobject_2_0_0!g_signal_has_handler_pending+0xf55
0022f3c8 62757e86 libgobject_2_0_0!g_signal_emit_valist+0x7ee
0022f3e8 60498320 libgobject_2_0_0!g_signal_emit+0x26
0022f408 6049a1dc libgtk_win32_2_0_0!gtk_button_clicked+0x40
0022f418 62743935 libgtk_win32_2_0_0!gtk_button_get_alignment+0x52c
0022f478 62756862 libgobject_2_0_0!g_closure_invoke+0x115
0022f568 62757c0e libgobject_2_0_0!g_signal_has_handler_pending+0x952
0022f7e8 62757e86 libgobject_2_0_0!g_signal_emit_valist+0x7ee
0022f808 604982a0 libgobject_2_0_0!g_signal_emit+0x26
0022f828 60499642 libgtk_win32_2_0_0!gtk_button_released+0x40
0022f838 60570ca2 libgtk_win32_2_0_0!gtk_button_set_relief+0xea2
0022f868 62743935 libgtk_win32_2_0_0!gtk_marshal_VOID__UINT_STRING+0x142
0022f8c8 62756a96 libgobject_2_0_0!g_closure_invoke+0x115
0022f9b8 6275796c libgobject_2_0_0!g_signal_has_handler_pending+0xb86
0022fc38 62757e86 libgobject_2_0_0!g_signal_emit_valist+0x54c
0022fc58 6066c2b4 libgobject_2_0_0!g_signal_emit+0x26
0022fc88 6056de11 libgtk_win32_2_0_0!gtk_widget_activate+0x224
0022fcb8 6056f13d libgtk_win32_2_0_0!gtk_propagate_event+0xd1
0022fd08 00a0f16e libgtk_win32_2_0_0!gtk_main_do_event+0x24d
0022fd28 672d8273 libgdk_win32_2_0_0!gdk_event_get_graphics_expose+0x3b1e
0022fd78 672d972b libglib_2_0_0!g_main_context_dispatch+0x173
0022fdb8 672d990a libglib_2_0_0!g_main_context_acquire+0x3db
0022fde8 00955eb2 libglib_2_0_0!g_main_loop_run+0x16a
0022fe08 0040dc7d libbonobo_2_0!bonobo_main+0x54
0022ff78 004011e7 image00400000+0xdc7d
0022ffb0 00401238 image00400000+0x11e7
0022ffc0 7c816d4f image00400000+0x1238
0022fff0 00000000 kernel32!BaseProcessStart+0x23
Comment 1 André Klapper 2006-07-21 17:24:30 UTC
i'm feeling free to add tor to the CC here. :-)
Comment 2 Tor Lillqvist 2006-07-21 18:05:14 UTC
Thanks for the bug report, and especially for the sample data (as I don't have Thunderbird myself to generatea file to import with). Will look into this next week.

Harish: I assume I can reassign this to myself? Should we have a Win32 component for Win32-specific problems in Evo?
Comment 3 Karsten Bräckelmann 2006-07-21 22:01:43 UTC
Tor: Sure you can. But I can, too. Done. ;)


Regarding the Win32 component... The same points we discussed a while ago still stand. How much Win32 specific bugs do we have? Does this really justify adding a component?

What about the OS field? (Pretty useless generally, but probably pretty accurate in the Windows case for Evolution...)
Comment 4 Tor Lillqvist 2006-07-21 22:15:00 UTC
We don't have many Win32 specific bugs yet, but if the usage on Win32 increases, and people are told to report problems here, it would be nice if they could select the win32 component, as I assume most bugs they would report would be win32-specific. That is just a guess, though. Maybe indeed best to wait and see.
Comment 5 Tor Lillqvist 2006-07-27 16:13:47 UTC
This bug is caused by the issues associated with importing variables from DLLs. As such, using variables from DLLs works fine without any extra magic, at least when using gcc. With MSVC, things would be different, then one would need to decorate the declarations of such variable with dllexport or dllimport attributes. 

(When compiling the DLL, the declaration of variables that are exported from the DLL should have the attribute dllexport, and when compiling code that uses the variable from the DLL, it should have the dllimport attribute. I.e. an #ifdef mess. See GLib source for some examples how ugly it can get.)

In my opinion, for cleanliness and Win32 portability it is best to avoid having variables in the API of a shared library altogether. A purely functional API is best.

In this case the problem is caused by the mail_thread_queued variable. It is defined inside the libevolution-mail DLL, and it is referenced by the libevolution-mail-importers DLL. The libevolution-mail-importers DLL is built *before* libevolution-mail has been built, so libevolution-mail is one of the DLLs that have dummy "bootstrap" import libraries built in the win32 folder.

Now, for functions that are referenced from not yet built DLLs, the bootstrap import libraries work fine. But mail_thread_queued is a variable. The win32/libevolution-mail.def file would need to specify that (have the keyword "DATA" added on that line). This however then means that then the declaration and definition of mail_thread_queued would also have to be "properly" decorated with dllexport/dllimport. Ugliness.

The cleanest way around this issue, IMHO, is to define a function in libevolution-mail that returns the value of mail_thread_queued, and use that function from libevolution-mail-importers instead of the variable itself.

The attached patch (to the stable branch, in a checkout from maybe a month ago, so the ChangeLog diffs might be slightly off) fixes the bug, and importing the mbox file works as it should.

OK to commit to both branches? Would it be better to use the getter function cross-platform to avoid #ifdefs? (At least in the development branch)
Comment 6 Tor Lillqvist 2006-07-27 16:14:53 UTC
Created attachment 69745 [details] [review]
Suggested patch
Comment 7 Tor Lillqvist 2006-09-05 02:41:43 UTC
Somebody have a look at the patch and approve, please. It should be quite obvious that this doesn't affect Unix at all.
Comment 8 Srinivasa Ragavan 2006-10-14 09:10:53 UTC
Tor, AFAIK it doesnt affect unix. Please commit to head and stable. Looks fine to me.
Comment 9 Harish Krishnaswamy 2006-10-16 14:56:46 UTC
Tor : I missed all your comments/questions after the reassignment - I was not on the CC list. 
Anyways, committed the patch to the HEAD. Thanks a lot :-).
Comment 10 Harish Krishnaswamy 2006-10-16 14:59:33 UTC
Committed to the stable branch (gnome-2-16 - Evolution 2.8.x) also.