buggtkmm-utils - Bugs: bug #21008, Logging framework requires apps to...

 
 

bug #21008: Logging framework requires apps to initialize threading

Submitted by:  Jonathon Jongsma <jonner>
Submitted on:  Sun 09 Sep 2007 01:24:45 AM UTC  
 
Category: glibmm-utilsSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Marko Anastasov <markoa>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 10 Sep 2007 07:03:02 PM UTC, comment #9:

yeah, that sounds like a reasonable approach as long as it's clearly documented.

Jonathon Jongsma <jonner>
Mon 10 Sep 2007 06:53:32 PM UTC, comment #8:

OK, I applied the patch.

I think this is good trade-off now. I see your point about the possibility of late threading initialization, but that would be a programmer's mistake. Someone dealing with concurrency should now what he/she is doing and one the things will be that this logging framework is thread safe, if you do this initialization before using it. Which then allows other people to have a thread-less version, and in which case you don't have to do anything, which is nice.

I should definitely put a note about this on clearly visible place(s).

Marko Anastasov <markoa>
Project AdministratorIn charge of this item.
Mon 10 Sep 2007 03:48:42 PM UTC, comment #7:

here i go again... :)
I just sent a little cosmetic patch related to this (using git-send-email), hopefully that works ok. But now I'm thinking that maybe this is not the best idea. For example, with the changes that we made, it is now possible to instantiate a LogSink before we initialize other libraries such as Glib / threading (and if it's possible, somebody will do it). Now if they initialize threading after creating a LogSink, that LogSink will not be threadsafe since we only check for whether threads are supported at construction time. So maybe we should have just left it as it was and required an init() function... I'm not sure anymore.

Jonathon Jongsma <jonner>
Mon 10 Sep 2007 03:14:58 PM UTC, comment #6:

Right. Thanks for taking a look; I should've been careful.

Created commit b1bb3f7: Use the mutex and ostream in the same scope when thread_aware (bug #21008).

Marko Anastasov <markoa>
Project AdministratorIn charge of this item.
Mon 10 Sep 2007 03:24:53 AM UTC, comment #5:

I just looked at the new changed, and I think there's some issues with this:

if (m_thread_aware)
Glib::Mutex::Lock lock (*m_ostream_mutex) ;

I think this is roughly equivalent to:

if (m_thread_aware) {
Glib::Mutex::Lock lock (*m_ostream_mutex) ;
}

Which means that the scope of the lock is only inside the if statement. So after the if statement, the mutex will be unlocked and so the function will no longer be threadsafe. Right?

Jonathon Jongsma <jonner>
Sun 09 Sep 2007 04:59:09 PM UTC, comment #4:

commit d4225b1: Made thread-awareness in the logging framework optional (bug #21008).

Marko Anastasov <markoa>
Project AdministratorIn charge of this item.
Sun 09 Sep 2007 04:31:43 PM UTC, comment #3:

I have committed a small modification so that client applications do not require to call do_init() at all.

Ie Glib threading is now being initialized privately in LogStream::Priv ctor.

P.S.:

This is of course not quite what you suggested, which I now agree with. So, by default, no init functions will be required and the logging will be sequential. Otherwise, muteces will be used prior to any writing to an ostream.

Marko Anastasov <markoa>
Project AdministratorIn charge of this item.
Sun 09 Sep 2007 01:35:21 PM UTC, comment #2:

Oops, I guess I missed that. But in any case, it still seems a little bit heavy to require that that threading be initialized just for a logging framework, doesn't it? I wonder if we could use the Glib::thread_supported() [1] function and only create and lock the mutex if the app has initialized (and thus is using) threading already?

[1] http://gtkmm.org/docs/glibmm-2.4/docs/reference/html/group__Threads.html#gd59d126f6197035e6f6a31e7f4d87818

Jonathon Jongsma <jonner>
Sun 09 Sep 2007 12:49:27 PM UTC, comment #1:

Did you call Glib::Util::Initializer::do_init()?

LogSink locks a mutex before writing.

Marko Anastasov <markoa>
Project AdministratorIn charge of this item.
Sun 09 Sep 2007 01:24:45 AM UTC, original submission:

When adding some logging to an app I'm working on, My application crashed while trying to create a Mutex with the following error:
GLib-ERROR **: The thread system is not yet initialized.

The stack trace is as follows:

#0 __kernel_vsyscall () at :0
#1 raise () at :0
#2 abort () at :0
#3 IA__g_logv () at gmessages.c:497
#4 IA__g_log () at gmessages.c:517
#5 g_thread_fail () at gthread.c:547
#6 Glib::Mutex::Mutex () at :0
#7 LogStream () at log-stream.cc:67
#8 Glib::Util::LogStream::default_log_stream () at log-stream.cc:339
#9 agave::ColorScale::render_scale () at colorscale.cc:417
#10 agave::ColorScale::on_expose_event () at colorscale.cc:317
#11 Gtk::Widget_Class::expose_event_callback () at :0
#12 _gtk_marshal_BOOLEAN__BOXED () at gtkmarshalers.c:84
#13 g_type_class_meta_marshal () at gclosure.c:567
#14 IA__g_closure_invoke () at gclosure.c:490
#15 signal_emit_unlocked_R () at gsignal.c:2478
#16 IA__g_signal_emit_valist () at gsignal.c:2209
#17 IA__g_signal_emit () at gsignal.c:2243
#18 gtk_widget_event_internal () at gtkwidget.c:3915
#19 IA__gtk_main_do_event () at gtkmain.c:1556
#20 gdk_window_process_updates_internal () at gdkwindow.c:2338
#21 IA__gdk_window_process_all_updates () at gdkwindow.c:2401
#22 gdk_window_update_idle () at gdkwindow.c:2259
#23 g_idle_dispatch () at gmain.c:3928
#24 IA__g_main_context_dispatch () at gmain.c:2045
#25 g_main_context_iterate () at gmain.c:2677
#26 IA__g_main_loop_run () at gmain.c:2881
#27 IA__gtk_main () at gtkmain.c:1177
#28 Gtk::Main::run_impl () at :0
#29 Gtk::Main::run () at :0
#30 main () at test-all.cc:91

It seems that we should not require applications to initialize threading just to use a logging framework.

Jonathon Jongsma <jonner>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by markoa (Posted a comment)
  • -unavailable- added by jonner (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 16 Sep 2007 06:31:31 PM UTCmarkoaOpen/ClosedOpen=>Closed
    Sun 16 Sep 2007 06:31:04 PM UTCmarkoaStatusIn Progress=>Fixed
    Sun 09 Sep 2007 12:58:22 PM UTCmarkoaCategoryNone=>glibmm-utils
    Sun 09 Sep 2007 12:49:27 PM UTCmarkoaStatusNone=>In Progress
      Assigned toNone=>markoa

    Back to the top


    Powered by Savane 3.1-cleanup1