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.
|
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).
|
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.
|
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).
|
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?
|
Sun 09 Sep 2007 04:59:09 PM UTC, comment #4:
commit d4225b1: Made thread-awareness in the logging framework optional (bug #21008).
|
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.
|
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
|
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.
|
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.
|