[AD] [ alleg-Bugs-1692717 ] some bugs

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]


Bugs item #1692717, was opened at 2007-04-02 10:25
Message generated for change (Comment added) made by mmimica
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105665&aid=1692717&group_id=5665

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: erno (scherno2000)
Assigned to: Nobody/Anonymous (nobody)
Summary: some bugs

Initial Comment:
1. rest(0) does not yield.
2. in fbcon.c console_users is incremented twice, but decremented only once.
3. If allegro application is run in console mode, and get_tty() returns 0 (running application from midnight commander), problems with threads created before fork().
4. If allegro application is run in console mode, from midnight commander, then the shell thinks the application is ended too early because of main process is killed in allegro initialization.

Please read the readme file in attached zip file.


----------------------------------------------------------------------

Comment By: Milan Mimica (mmimica)
Date: 2007-04-05 14:52

Message:
Logged In: YES 
user_id=1171214
Originator: NO

1st bug: For some reason (unknown to me) allegro 4.3 uses a simplified
implementantion of rest(), as opposed to 4.2's rest(), which uses
sched_yield() if available. You're right, this should be fixed.

2nd bug: I am going to commit the fix. How do I sign you?

3rd and 4th: this only happens when running from mc. I asked at the forums
once why allergo programs cannot be ran from screen (console virtual
screens manager). I was told that allegro programs need a real console. I
guess the same applies to mc. Besides, I couldn't make it work even with
your fixes.

----------------------------------------------------------------------

Comment By: erno (scherno2000)
Date: 2007-04-02 14:57

Message:
Logged In: YES 
user_id=1542934
Originator: YES

The problem is that the background thread __al_linux_bgman_init() is
called in sys_linux_init(), that function calls bg_man_pthreads_init()
where is created the background thread. This is needed by the sound system
too. The sound will not work too if the background thread is created before
the fork. The problem with fork() is that the global variables used by
threads created are in the context of the main process, but after the
fork() the new function calls are using the new global variables.
It isn't enough to put the __al_linux_use_console() in install_timer()
because of the background thread.
Now the fork happens in keyboard_init() or the al_linux_console_graphics
call in fbcon.c, too late.

I didn't studied the source too deeply. Why it is necessary to call fork()
if one branch is immediately exited? If this is necessary can we exit from
child process instead of parent?


----------------------------------------------------------------------

Comment By: Milan Mimica (mmimica)
Date: 2007-04-02 13:43

Message:
Logged In: YES 
user_id=1171214
Originator: NO

The 2nd bug, fix in fbcon.c: OK

The 3rd bug, lsystem.c and lconsole.c: We used to init the console in
sys_linux_init(), like your fix does, but we (me) didn't want the program
to be attached to a console unless really necessary. We delayed the console
initialisation until set_gfx_mode() or install_keyboard() was called (hence
the whole console_users thing)... Now it turns out that the console is
needed because of some fork() call... can you find another way to fix this?
Could we put __al_linux_use_console() call in install_timer() like we do on
keyboard init? Where does this fork() happen exactly?



----------------------------------------------------------------------

Comment By: erno (scherno2000)
Date: 2007-04-02 12:25

Message:
Logged In: YES 
user_id=1542934
Originator: YES

If you want you can see the differences rapidly, if you search the word
ERNO in the files from zip. All of differences are active
only if ERNO is defined.
I didn't sent diffs because I wanted you to check if my modifications are
all correct, but I can send diffs if you want.


I just changed the al_rest() function like this:
when msecs<0 it calls explicitly sched_yield() instead of select(...)
to force yielding from the showa16.cpp with rest(-1), and not change
functionality if the parameter >=0.
I used the #define to keep the old functionality if I comment the 

#define ERNO 

line

the modified function:

#define ERNO
void al_rest(long msecs)
{
#ifdef ALLEGRO_MACOSX
   usleep(msecs * 1000);
#else
   struct timeval timeout;
#ifdef ERNO
   if(msecs<0)
   {
     sched_yield();
   }
   else
#endif
   { 
     timeout.tv_sec = 0;
     timeout.tv_usec = msecs * 1000;
     select(0, NULL, NULL, NULL, &timeout);
   }
#endif
}

You can check showa16.cpp modifying the rest parameter to -1, 0, 1 and
commenting rest.
With rest -1 and 1 the result will be the same (only 1 thread running)
With rest 0 and without rest results will be the same, and half value of
previous (2 threads running).


----------------------------------------------------------------------

Comment By: Peter Wang (tjaden)
Date: 2007-04-02 11:16

Message:
Logged In: YES 
user_id=28616
Originator: NO

Please generate a diffs for each change and send those if possible. It's
difficult to see what's going on and apply your changes. Thanks.


----------------------------------------------------------------------

Comment By: Elias Pschernig (elias)
Date: 2007-04-02 11:07

Message:
Logged In: YES 
user_id=32894
Originator: NO

I'll look at the .zip later. About utime.c - what did you modify and why?

----------------------------------------------------------------------

Comment By: erno (scherno2000)
Date: 2007-04-02 11:03

Message:
Logged In: YES 
user_id=1542934
Originator: YES

In the zip file is an example you can test:

showa16.cpp
.
The problem is, that select(...) with timeout == 0 does not yield to the
other thread. (See the select man page),
but the allegro documentation says that. If the timeout is <> 0 every
thing is OK. If I am calling sched_yield() instead of
select with timeout==0 every thing is OK. I tested this with the modified
version of utime.c (in the zip file) too, and changed the rest parameter
with -1.
 

----------------------------------------------------------------------

Comment By: Elias Pschernig (elias)
Date: 2007-04-02 10:32

Message:
Logged In: YES 
user_id=32894
Originator: NO

I don't know about console mode, but about rest(0), what makes you believe
what you said? From the sources (src/unix/usystem.c), this is the function
called when you do rest(0) in unix:

void _unix_yield_timeslice(void)
{
   #if defined(ALLEGRO_USE_SCHED_YIELD) &&
defined(_POSIX_PRIORITY_SCHEDULING)

      sched_yield();

   #else

      struct timeval timeout;
      timeout.tv_sec = 0;
      timeout.tv_usec = 0;
      select(0, NULL, NULL, NULL, &timeout);

   #endif
}

So normally it directly calls sched_yield (e.g. in linux), and if that's
not available, it will do a select with time 0, which also will call back
to the scheduler and give other programs a chance to run. Note that
yielding does not imply giving up any CPU time in case you expected that..

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105665&aid=1692717&group_id=5665




Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/