[Kde-pim] [patch] handling of static data in akonadi and mailtransport

[Ingo Klöcker]

On Tuesday 20 May 2008, Jarosław Staniek wrote:
> Ingo Klöcker said the following, On 2008-05-20 00:14:
> > On Monday 19 May 2008, Jarosław Staniek wrote:
> >> Index: mailtransport/socket.cpp
> >> ==================================================================
> >>= --- mailtransport/socket.cpp    (wersja 809997)
> >> +++ mailtransport/socket.cpp    (kopia robocza)
> >> @@ -54,6 +54,8 @@
> >>        void slotModeChanged( QSslSocket::SslMode  state );
> >>        void slotSocketRead();
> >>        void slotSslErrors( const QList<QSslError> &errors );
> >> +    private:
> >> +      QString msg;
> >
> > This should be
> > +      QString m_msg;
> >
> >>    };
> >>  }
> >>
> >> @@ -102,19 +104,19 @@
> >>      return;
> >>    }
> >>
> >> -  static QString msg;
> >> -  msg += QLatin1String( socket->readAll() );
> >> +  QString m_msg;
> >
> > The above line has to be deleted.
>
> Sorry I must have been tired too. Attached updated patch.
> Till, Bernhard: this is the newest patch to apply (after removing the
> old one).
>
> >> +  m_msg += QLatin1String( socket->readAll() );
> >>
> >> -  if ( !msg.endsWith( QLatin1Char( '\n' ) ) ) {
> >> +  if ( !m_msg.endsWith( QLatin1Char( '\n' ) ) ) {
> >>      return;
> >>    }
> >>
> >>  #ifdef comm_debug
> >> -  kDebug() << socket->isEncrypted() << msg.trimmed();
> >> +  kDebug() << socket->isEncrypted() << m_msg.trimmed();
> >>  #endif
> >>
> >> -  emit q->data( msg );
> >> -  msg.clear();
> >> +  emit q->data( m_msg );
> >> +  m_msg.clear();
> >>  }
> >
> > Concerning mailtransport/smtpjob.cpp I'm wondering whether
> > SlavePool can be destroyed before the last SmtpJob is destroyed. I
> > guess it would be saner to make the last SmtpJob destroy the slave
> > pool. The pool is anyway cleared each time the reference count
> > drops to 0, so the slave pool could as well be destroyed at that
> > point. And in the c'tor of SmtpJob the pool would be created if it
> > does not exist. I don't think K_GLOBAL_STATIC is the correct tool
> > for the job, but I'm too tired to really think about it right now.
>
> This woud be extra code and allocation to save perhaps 10 bytes.

This isn't about saving a few bytes. If the global static s_slavePool is 
destroyed before the last SmtpJob is destroyed then accessing 
s_slavePool in SmtpJob::~SmtpJob() will cause a crash at program exit 
(or, more precisely, it will cause a qFatal()).

I have no idea when those SmtpJobs are destroyed if they are still 
running on program exit, so I don't know whether this crash can 
actually happen. But better safe than sorry.

So please add

  if ( ! s_slavePool.isDestroyed() ) {
    // okay, it is safe to access s_slavePool
    ...
  }

(without the comment) everywhere in SmtpJob where s_slavePool is 
accessed.


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20080520/dd9c9fe2/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/