[RFC] persistence of schedlock mode

[RFC] persistence of schedlock mode

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

Bug 11580 points out a problem related to persistence of the
scheduler locking mode.  If you set the mode to "on" (which
means that only one thread can run), and then kill and restart
your program, the mode persists and your program may deadlock.

I don't think this is a problem for mode "step", and it could
be argued that mode "on" is "use at your own risk".  However
I've been thinking about how to resolve it, and this simple
but intrusive patch is what I come up with.

Comments?


[-- Attachment #2: schedlock.txt --]
[-- Type: text/plain, Size: 1236 bytes --]

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.438
diff -u -p -r1.438 infrun.c
--- infrun.c	6 May 2010 19:14:08 -0000	1.438
+++ infrun.c	12 May 2010 17:37:09 -0000
@@ -1416,6 +1416,18 @@ set_schedlock_func (char *args, int from
     }
 }
 
+/* reset_schedlock -- public */
+ 
+void
+reset_schedlock ()
+{
+  if (scheduler_mode == schedlock_on)
+    {
+      warning ("Resetting scheduler-lock mode to 'off'");
+      scheduler_mode = schedlock_off;
+    }
+}
+
 /* True if execution commands resume all threads of all processes by
    default; otherwise, resume only threads of the current inferior
    process.  */
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.250
diff -u -p -r1.250 target.c
--- target.c	6 May 2010 22:29:49 -0000	1.250
+++ target.c	12 May 2010 17:37:09 -0000
@@ -2231,6 +2231,10 @@ void
 target_mourn_inferior (void)
 {
   struct target_ops *t;
+
+  /* Clear schedlock in infrun.c */
+  reset_schedlock ();
+
   for (t = current_target.beneath; t != NULL; t = t->beneath)
     {
       if (t->to_mourn_inferior != NULL)	

Re: [RFC] persistence of schedlock mode

[Pedro Alves]

On Wednesday 12 May 2010 19:08:48, Michael Snyder wrote:
> Bug 11580 points out a problem related to persistence of the
> scheduler locking mode.  If you set the mode to "on" (which
> means that only one thread can run), and then kill and restart
> your program, the mode persists and your program may deadlock.
> 
> I don't think this is a problem for mode "step", and it could
> be argued that mode "on" is "use at your own risk".  However
> I've been thinking about how to resolve it, and this simple
> but intrusive patch is what I come up with.
> 
> Comments?

I think this is not a good idea.  (I added a larger comment on
the PR before noticing this email).

-- 
Pedro Alves

Re: [RFC] persistence of schedlock mode

[Mark Kettenis]

> Date: Wed, 12 May 2010 11:08:48 -0700
> From: Michael Snyder <msnyder@vmware.com>
> 
> Bug 11580 points out a problem related to persistence of the
> scheduler locking mode.  If you set the mode to "on" (which
> means that only one thread can run), and then kill and restart
> your program, the mode persists and your program may deadlock.
> 
> I don't think this is a problem for mode "step", and it could
> be argued that mode "on" is "use at your own risk".  However
> I've been thinking about how to resolve it, and this simple
> but intrusive patch is what I come up with.
> 
> Comments?

Ah, that's clever.  The user will be notified, but doesn't have to
answer an additional question.

> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.438
> diff -u -p -r1.438 infrun.c
> --- infrun.c	6 May 2010 19:14:08 -0000	1.438
> +++ infrun.c	12 May 2010 17:37:09 -0000
> @@ -1416,6 +1416,18 @@ set_schedlock_func (char *args, int from
>      }
>  }
>  
> +/* reset_schedlock -- public */
> + 
> +void
> +reset_schedlock ()
> +{
> +  if (scheduler_mode == schedlock_on)
> +    {
> +      warning ("Resetting scheduler-lock mode to 'off'");
> +      scheduler_mode = schedlock_off;
> +    }
> +}
> +
>  /* True if execution commands resume all threads of all processes by
>     default; otherwise, resume only threads of the current inferior
>     process.  */
> Index: target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.250
> diff -u -p -r1.250 target.c
> --- target.c	6 May 2010 22:29:49 -0000	1.250
> +++ target.c	12 May 2010 17:37:09 -0000
> @@ -2231,6 +2231,10 @@ void
>  target_mourn_inferior (void)
>  {
>    struct target_ops *t;
> +
> +  /* Clear schedlock in infrun.c */
> +  reset_schedlock ();
> +
>    for (t = current_target.beneath; t != NULL; t = t->beneath)
>      {
>        if (t->to_mourn_inferior != NULL)	

RE: [RFC] persistence of schedlock mode

[Pierre Muller]

  Did I miss some part of the discussion here?

  I really have the impression that this
patch went in by error in a long series of patches 
from Michael all labeled with '[ob]... white space'
without the patch itself being approved on the main thread. 

 First reset_schedlock went in and generated a 'New ARI warning'
Jan replied to it
http://sourceware.org/ml/gdb-patches/2010-05/msg00320.html

  Then, another commit of the same series
http://sourceware.org/ml/gdb-cvs/2010-05/msg00145.html
added a call to reset_schedlock in infrun.c.
as can be seen in
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/target.c.diff?cvsroot=src&r
1=1.250&r2=1.251

This lead to a compilation error that Hui 
tried to fix, and Joel added a second patch to cleanup.

  I have no opinion of the validity of the patch itself, 
but the way it got into CVS leaves a strange impression to me.


Pierre Muller
Pascal language support maintainer for GDB



> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Michael Snyder
> Envoyé : Wednesday, May 12, 2010 8:09 PM
> À : gdb-patches@sourceware.org
> Objet : [RFC] persistence of schedlock mode
> 
> Bug 11580 points out a problem related to persistence of the
> scheduler locking mode.  If you set the mode to "on" (which
> means that only one thread can run), and then kill and restart
> your program, the mode persists and your program may deadlock.
> 
> I don't think this is a problem for mode "step", and it could
> be argued that mode "on" is "use at your own risk".  However
> I've been thinking about how to resolve it, and this simple
> but intrusive patch is what I come up with.
> 
> Comments?

Re: [RFC] persistence of schedlock mode

[Nathan Froyd]

On Wed, May 12, 2010 at 11:08:48AM -0700, Michael Snyder wrote:
> I don't think this is a problem for mode "step", and it could
> be argued that mode "on" is "use at your own risk".  However
> I've been thinking about how to resolve it, and this simple
> but intrusive patch is what I come up with.

You should add a declaration for reset_schedlock someplace; otherwise,
you get:

gdb/target.c: In function 'target_mourn_inferior':
gdb/target.c:2249: error: implicit declaration of function 'reset_schedlock'

-Nathan

Re: [RFC] persistence of schedlock mode

[Joel Brobecker]

> I really have the impression that this
> patch went in by error in a long series of patches 
> from Michael all labeled with '[ob]... white space'
> without the patch itself being approved on the main thread. 

Now sure how or why it got there, but I wanted to confirm that
all patches got reverted by Pedro (thanks Pedro!).

-- 
Joel

Re: [RFC] persistence of schedlock mode

[Michael Snyder]

Joel Brobecker wrote:
>> I really have the impression that this
>> patch went in by error in a long series of patches 
>> from Michael all labeled with '[ob]... white space'
>> without the patch itself being approved on the main thread. 
> 
> Now sure how or why it got there, but I wanted to confirm that
> all patches got reverted by Pedro (thanks Pedro!).
> 

Sorry, this was an accident.
Thanks Pedro for reverting it.
Michael