Fri 25 May 2007 09:43:25 AM UTC, comment #12:
I haven't seen the crash since the patch was committed, so i assume it's fixed.
|
Mon 21 May 2007 07:09:07 AM UTC, comment #11:
I committed that patch finally. At least the segfault is fixed.
|
Sun 20 May 2007 06:59:11 AM UTC, comment #10:
Ok, would a log(any, arg, type) work for you then ?
It would be semantically equivalent of current 'dbglogfile << any << arg << type << endl' but force specifying exact boundaries for a call, making locking more manageable.
I still think internationalization would suffer, but we're looking for a compromise here (and we can keep both interfaces)
|
Sun 20 May 2007 01:29:46 AM UTC, comment #9:
Cout doesn't write timestamps with each line, nor does it optionally write to a disk file. Using printf() style output in C++ programs is just plain gross. To me add .c_str() is much messier than looking for %s.
|
Sat 19 May 2007 11:25:44 PM UTC, comment #8:
Just another drawback of output operator jumped to my mind while about to sleep: internationalization !
Different languages construct phrases placing elements in different order. So, think about:
cout << "subject verb" << object;
And:
printf("subject verb %s", object);
The latter is more eas to translate, as the translator could
move the %s part everywhere !!
This hitted my attention while thinking about the log(any, argument, here) proposal, which would make translation an hard job... not a problem if log(any, thing, here) would be only used for temporary debugging (not in releases) but again, I don't see why not using std::cout in that case...
|
Sat 19 May 2007 11:03:28 PM UTC, comment #7:
The patch seems to fix the segfault, but since it didn't always appear, i can't say for certain it's fixed, but it does look like it.
|
Sat 19 May 2007 11:00:04 PM UTC, comment #6:
Last message before going to sleep.
My proposal is to commit this patch and work on a new interface
for logging which accept arbitrary arguments.
The new interface would look like this:
log_*(any, argument, here);
The '*' would follow any existing logging function name.
The advantage of it would be not having to deal with
printf-like formats.
To avoid the clashes it could be a member of LogFile so
migration is smooth.
NOTE that this would require defining a templated function
for any number of arguments we want to pass to the log()
method, which could be maken more maintainable by using
metaprogramming tecniques, which boost provide.
See: http://www.boost.org/libs/preprocessor/doc/index.html
Of course I'm suggesting we commit my patch before 0.8.0
and work on the more general logging facilities after it.
Comments welcome.
|
Sat 19 May 2007 09:56:40 PM UTC, comment #5:
Sorry, reattached (previous patch failed printing log_debug messages)
(file #12806)
|
Sat 19 May 2007 09:52:06 PM UTC, comment #4:
The LogFile::operator<< interface has these drawbacks:
1) A debugging line takes multiple function calls, thus making it hard to manage locking or labelling.
2) It doesn't extend std::ostream::operator<<, thus isn't useful to use pre-existing output operators.
Attached is a patch that makes LogFile::operator<< methods private and expose a new LogFile::log() method, intended to be called by the existing log_* functions, which are semantically defined and documented.
The need expressed by rob is not bothering about arguments type.
I think we might implement this with some templates magic, altought I don't have a plan currently.
So, please review the patch. Most importantly see if it fixes the thread-safety issue.
(file #12805)
|
Sat 19 May 2007 08:30:16 PM UTC, comment #3:
To be honest, I dislike printf() style debugging in C++ programs, so in a way I'd prefer to see us use dbglogfile << more than all the log_() functions. We can derive other classes from the base class to have different output streams instead of all the log_() functions. It's not a practical short term goal, but someday I'd prefer to eliminate log_().
|
Sat 19 May 2007 08:24:15 PM UTC, comment #2:
More info on this task:
The problem is that some files are still using operator<< of LogFile, which would still be non thread-safe if we move locking
to the log_* functions.
A plan could be:
1) Forbid use of operator<< making them private methods of the LogFile class
2) Provide a public log(level, fmt, ...) methods of LogFile, locking a private io_mutex
3) Updating any user so that operator<< use is substituted by appropriate call to log_*()
4) Have log_*() functions call LogFile::log(level, fmg, ...)
|
Sat 19 May 2007 08:11:38 PM UTC, comment #1:
Took a look, it seems all operator<< use a mutex, except
one, the templated:
template <class T>
LogFile& operator << (const T& any)
Generally speaking... I'd have the log_* functions use
the mutex, rather then the operator<<, which should likely
never be directly invoked.
Using the mutex from the templated function won't be easy
as the io_mutex is defined in the .cpp file, not the .h one.
Feel like trying ? (remove all scoped_lock(io_mutex) from operator<< and add to log_*)
|
Sat 19 May 2007 02:01:53 PM UTC, original submission:
I'm working on NetStreamFfmpeg, and are using log_debug() (a lot) to debug, and sometimes i get segfaults. I'm using threads, so maybe it's not thread-safe.
Backtrace:
[Switching to Thread 47507331171776 (LWP 613)]
0x00002b3523102665 in raise () from /lib/libc.so.6
(gdb) bt
#0 0x00002b3523102665 in raise () from /lib/libc.so.6
#1 0x00002b352310391e in abort () from /lib/libc.so.6
#2 0x00002b3523137f96 in __libc_message () from /lib/libc.so.6
#3 0x00002b352313cf3d in malloc_printerr () from /lib/libc.so.6
#4 0x00002b352313e576 in free () from /lib/libc.so.6
#5 0x00002b3522e62afc in std::string::assign () from /usr/lib/libstdc++.so.6
#6 0x00002b3522b3b0cc in gnash::LogFile::operator<< (this=0x2b3522c95840,
str=0x2b3522b76964 "DEBUG: ")
at /home/tomas/gnash/gnash/libbase/log.cpp:595
#7 0x00002b3522b3b428 in gnash::log_debug (
fmt=0x2b352293fa88 "breaking loop, queue size: %ld, current: %lf, video: %lf") at /home/tomas/gnash/gnash/libbase/log.cpp:210
#8 0x00002b35228ba09a in gnash::NetStreamFfmpeg::advance (this=0x7e0470)
at /home/tomas/gnash/gnash/server/asobj/NetStreamFfmpeg.cpp:1102
#9 0x00002b35227ee942 in gnash::video_stream_instance::advance (this=0x7da5e0)
at /home/tomas/gnash/gnash/server/video_stream_instance.cpp:126
|