Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

[Dan Williams <dcbw redhat com>]

On Tue, 2012-11-27 at 13:19 -0800, Ben Chan wrote:
> Yes, it's related to the data_available (mm-serial-port.c:767) crash
> (crosbug.com/35391).  I'm running suspend/resume stress test with
> ModemManager under valgrind.

Any way to get MM running with --debug in those logs?  Also, in your
sources, does line 767 match up with:

    info = g_queue_peek_nth (priv->queue, 0);

or some other line?

One other thing to do, put an

mm_info ("(%s): disposing", mm_port_get_device (MM_PORT (self)));

into dispose() and lets see when it gets disposed and if anything tries
to open it during/after dispose.  Drop another of these into
mm_serial_port_new().

Dan

> 
> Thanks,
> Ben
> 
> On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado
> <aleksander lanedo com> wrote:
>         On 11/27/2012 09:39 PM, Ben Chan wrote:
>         > ---
>         >  src/mm-serial-port.c |   10 ++++++++++
>         >  1 files changed, 10 insertions(+), 0 deletions(-)
>         
>         
>         
>         Don't think it's the correct approach.
>         
>         I don't think we ever run g_object_run_dispose() ourselves for
>         port
>         objects, and that means that whenever we got the port object
>         disposed it
>         was because it was the last valid reference.
>         
>         And that means that you won't be able to read priv->disposed
>         afterwards
>         as that would mean reading already freed memory.
>         
>         There clearly is a missing reference around, or a timeout or
>         other
>         source which has the port as user_data and is not being
>         properly cleaned
>         up (e.g. removing the timeout or event source when the port is
>         disposed). Running it under valgrind should confirm this. Is
>         this
>         related to the data_available() crashes during suspend/resume?
>         
>         
>         >
>         > diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
>         > index 0a8820d..a33c745 100644
>         > --- a/src/mm-serial-port.c
>         > +++ b/src/mm-serial-port.c
>         > @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
>         >
>         >  typedef struct {
>         >      guint32 open_count;
>         > +    gboolean disposed;
>         >      gboolean forced_close;
>         >      int fd;
>         >      GHashTable *reply_cache;
>         > @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort
>         *self, GError **error)
>         >
>         >      device = mm_port_get_device (MM_PORT (self));
>         >
>         > +    /* If the MMSerialPort object has been disposed, just
>         return an error. */
>         > +    if (priv->disposed) {
>         > +        mm_info ("(%s) skipped opening serial port that has
>         been disposed", device);
>         > +        return FALSE;
>         > +    }
>         > +
>         >      if (priv->open_count) {
>         >          /* Already open */
>         >          goto success;
>         > @@ -1537,6 +1544,9 @@ dispose (GObject *object)
>         >  {
>         >      MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE
>         (object);
>         >
>         > +    /* Mark the MMSerialPort object as disposed to prevent
>         it from being re-opened. */
>         > +    priv->disposed = TRUE;
>         > +
>         >      if (priv->timeout_id) {
>         >          g_source_remove (priv->timeout_id);
>         >          priv->timeout_id = 0;
>         >
>         
>         
>         --
>         
>         Aleksander
> 
> 
> _______________________________________________
> networkmanager-list mailing list
> networkmanager-list gnome org
> https://mail.gnome.org/mailman/listinfo/networkmanager-list