[Kos-dev] "Race condition" 1
Thomas Petazzoni
kos-dev@enix.org
Sun, 25 May 2003 21:19:52 +0200
This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enigB6345114B1BCE1FBED641A20
Content-Type: multipart/mixed;
boundary="------------020907010807010603010300"
This is a multi-part message in MIME format.
--------------020907010807010603010300
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit
Hello,
J'ai trouvé un premier problème dans le code qu'on a pondu avec David.
Dans le fichier x86/task/cpl0_switch.c, la fonction
cpl0_switch_no_return marque le thread passé en paramètre comme étant à
détruire (en faisant pointer la variable globale thread_to_be_destroyed
dessus). Le thread était alors effectivement supprimé lors du
cpl0_switch_with_return.
Or, dans cpl0_switch_no_return, on retourne dans le thread en faisant un
"iret", donc clairement les interruptions peuvent être activées, et on
peut se faire interrompre à ce moment là. (Rappel =
thread_to_be_destroyed est non NULL, il y a donc un thread à détruire).
Si, pile à ce moment là, un thread se termine, cpl0_switch_no_return est
appelé, et l'ASSERTion vérifiant que thread_to_be_destroyed est NULL échoue.
Evidemment, je n'ai pas trouvé ça par inspection du code source, mais en
faisant des tests avec plein de threads qui se terminaient dans tous les
sens.
Voila la solution que j'ai trouvée :
* la suppression du thread se fait via une fonction
cpl0_delete_pending_thread (et non pas dans la fonction
cpl0_switch_with_return)
* la fonction cpl0_delete_pending_thread est appelée depuis le code
assembleur de cpl0_switch_no_return_internal dès qu'on a changé de pile,
mais avant de restaurer le contexte.
Ci joint le patch à appliquer (sur x86/task/).
David, qu'en penses-tu ?
Thomas
--
PETAZZONI Thomas - thomas.petazzoni@enix.org - UIN : 34937744
http://www.enix.org/~thomas/
KOS: http://kos.enix.org/ - Lolut: http://lolut.utbm.info
Fingerprint : 0BE1 4CF3 CEA4 AC9D CC6E 1624 F653 CB30 98D3 F7A7
--------------020907010807010603010300
Content-Type: text/plain;
name="x86_task_thread_deletion.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="x86_task_thread_deletion.patch"
ndex: modules/x86/task/_cpl0_switch.c
===================================================================
RCS file: /var/cvs/kos/kos/modules/x86/task/_cpl0_switch.c,v
retrieving revision 1.2
diff -u -u -r1.2 _cpl0_switch.c
--- modules/x86/task/_cpl0_switch.c 25 May 2003 14:32:13 -0000 1.2
+++ modules/x86/task/_cpl0_switch.c 25 May 2003 19:05:38 -0000
@@ -9,26 +9,28 @@
void cpl0_switch_no_return (cpu_state_t *cpu_state,
thread_t *myself)
{
- __dbg_printk("3\n");
ASSERT_FATAL(test_interrupt_enabled() == FALSE);
ASSERT_FATAL(get_hw_isr_nested_level() == 0);
ASSERT_FATAL(thread_to_be_destroyed == NULL);
thread_to_be_destroyed = myself;
- __dbg_printk("4\n");
cpl0_switch_no_return_internal(cpu_state);
}
-void cpl0_switch_with_return (cpu_state_t **from_context,
- cpu_state_t *to_context)
+void cpl0_delete_pending_thread()
{
- ASSERT_FATAL(test_interrupt_enabled() == FALSE);
- ASSERT_FATAL(get_hw_isr_nested_level() == 0);
- cpl0_switch_with_return_internal(from_context, to_context);
-
if (thread_to_be_destroyed != NULL)
{
thread_t *thread_to_destroy = thread_to_be_destroyed;
thread_to_be_destroyed = NULL;
delete_kernel_thread(thread_to_destroy);
}
+}
+
+void cpl0_switch_with_return (cpu_state_t **from_context,
+ cpu_state_t *to_context)
+{
+ ASSERT_FATAL(test_interrupt_enabled() == FALSE);
+ ASSERT_FATAL(get_hw_isr_nested_level() == 0);
+ // ASSERT_FATAL(thread_to_be_destroyed == NULL);
+ cpl0_switch_with_return_internal(from_context, to_context);
}
Index: modules/x86/task/_cpl0_switch_asm.S
===================================================================
RCS file: /var/cvs/kos/kos/modules/x86/task/_cpl0_switch_asm.S,v
retrieving revision 1.1
diff -u -u -r1.1 _cpl0_switch_asm.S
--- modules/x86/task/_cpl0_switch_asm.S 25 May 2003 12:27:39 -0000 1.1
+++ modules/x86/task/_cpl0_switch_asm.S 25 May 2003 19:05:38 -0000
@@ -7,7 +7,7 @@
*/
#define ASM_SOURCE 1
- .file "_cpl0_switch.S"
+ .file "_cpl0_switch_asm.S"
/*
* Assembler source for -fomit-frame-pointer considerations, since gcc
@@ -15,7 +15,9 @@
*/
.text
-
+
+.extern cpl0_delete_pending_thread
+
/* The interrupts are set in the state in which they were set by the
to_context (no brute force sti) */
@@ -27,6 +29,7 @@
// caller ip -- esp
movl 0x4(%esp),%eax
movl %eax,%esp
+ call cpl0_delete_pending_thread
popw %gs
popw %fs
popw %es
Index: modules/x86/task/_thread_cpu_context.c
===================================================================
RCS file: /var/cvs/kos/kos/modules/x86/task/_thread_cpu_context.c,v
retrieving revision 1.9
diff -u -u -r1.9 _thread_cpu_context.c
--- modules/x86/task/_thread_cpu_context.c 25 May 2003 14:32:13 -0000 1.9
+++ modules/x86/task/_thread_cpu_context.c 25 May 2003 19:05:38 -0000
@@ -17,7 +17,6 @@
static void execute_thread(kernel_thread_fct_t *kt, void *data)
{
- char a;
__dbg_printk("kt=0x%x, data=0x%x\n", kt, data);
kt(data);
reschedule_after_termination();
--------------020907010807010603010300--
--------------enigB6345114B1BCE1FBED641A20
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)
Comment: Using GnuPG with Debian - http://enigmail.mozdev.org
iD8DBQE+0RdY9lPLMJjT96cRAgusAKCkqoFalHGh5oa/T9a6ce7fUHvvRACfdEd1
b/yz43Vc5m9DPDds6V//O4U=
=tEYd
-----END PGP SIGNATURE-----
--------------enigB6345114B1BCE1FBED641A20--