[PATCH] Fix QWSLock "invalid argument" logs

[Neil Jerram]

There was no known actual problem associated with these logs, but they
were spamming the log, so I thought it worth trying to understand and
fix them.

The confusion is that there are two different ways of creating QWSLock
objects.  "QWSLock()" creates an object that creates a new set of
semaphores, whereas "QWSLock(id)" creates an object that aliases the
existing set of semaphores with ID id.  What seems to happen is that
each application creates a semaphore set scoped to that
application (QWSDisplay::Data::clientLock in qapplication_qws.cpp),
then this semaphore set is passed by complex means to
places (QWSClientPrivate and QWSMemorySurface) that use the semaphores
for a short period and then delete their QWSLock objects.

The problem was that the QWSLock destructor always destroyed the
semaphore set, even when that QWSLock hadn't create the semaphores
itself, hence making the semaphores invalid for other QWSLock objects
still referencing the same set.

Clearly a QWSLock object shouldn't destroy the semaphore set if it
didn't create it itself, and that is confirmed by the fact that one of
the implementations inside QWSLock already implements this logic, with
the 'owned' flag.  The fix is to implement this for the #ifndef
QT_POSIX_IPC case - which is what is used in QtMoko - just as is
already implemented for the #ifdef QT_POSIX_IPC case.
---
 src/gui/embedded/qwindowsystem_qws.cpp  |    3 +++
 src/gui/embedded/qwslock.cpp            |   17 +++++++++++++----
 src/gui/embedded/qwslock_p.h            |    2 +-
 src/gui/kernel/qapplication_qws.cpp     |    2 ++
 src/gui/painting/qwindowsurface_qws.cpp |    2 ++
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/gui/embedded/qwindowsystem_qws.cpp b/src/gui/embedded/qwindowsystem_qws.cpp
index a1c613d..1ed8b6d 100644
--- a/src/gui/embedded/qwindowsystem_qws.cpp
+++ b/src/gui/embedded/qwindowsystem_qws.cpp
@@ -680,6 +680,7 @@ QWSClientPrivate::QWSClientPrivate()
 QWSClientPrivate::~QWSClientPrivate()
 {
 #ifndef QT_NO_QWS_MULTIPROCESS
+    //qDebug("QWSClientPrivate::~QWSClientPrivate()");
     delete clientLock;
 #endif
 }
@@ -689,7 +690,9 @@ void QWSClientPrivate::setLockId(int id)
 #ifdef QT_NO_QWS_MULTIPROCESS
     Q_UNUSED(id);
 #else
+    //qDebug("QWSClientPrivate::setLockId(%d)", id);
     clientLock = new QWSLock(id);
+    //qDebug("=> %p", clientLock);
 #endif
 }
 
diff --git a/src/gui/embedded/qwslock.cpp b/src/gui/embedded/qwslock.cpp
index 9914a24..1055785 100644
--- a/src/gui/embedded/qwslock.cpp
+++ b/src/gui/embedded/qwslock.cpp
@@ -83,9 +83,13 @@ QWSLock::QWSLock(int id) : semId(id)
     QWSSignalHandler::instance()->addWSLock(this);
 #endif
 
+    owned = false;
+
 #ifndef QT_POSIX_IPC
     if (semId == -1) {
         semId = semget(IPC_PRIVATE, 3, IPC_CREAT | 0666);
+        owned = true;
+	//qDebug("QWSLock::QWSLock(): %p, %d", this, (int)semId);
         if (semId == -1) {
             perror("QWSLock::QWSLock");
             qFatal("Unable to create semaphore");
@@ -100,7 +104,6 @@ QWSLock::QWSLock(int id) : semId(id)
     }
 #else
     sems[0] = sems[1] = sems[2] = SEM_FAILED;
-    owned = false;
 
     if (semId == -1) {
         // ### generate really unique IDs
@@ -134,9 +137,12 @@ QWSLock::~QWSLock()
 
     if (semId != -1) {
 #ifndef QT_POSIX_IPC
-        qt_semun semval;
-        semval.val = 0;
-        semctl(semId, 0, IPC_RMID, semval);
+	if (owned) {
+	    qt_semun semval;
+	    semval.val = 0;
+	    semctl(semId, 0, IPC_RMID, semval);
+	}
+	//qDebug("QWSLock::~QWSLock(): %p, %d", this, (int)semId);
         semId = -1;
 #else
         // emulate the SEM_UNDO behavior for the BackingStore lock
@@ -170,8 +176,10 @@ bool QWSLock::up(unsigned short semNum)
     if (semNum == BackingStore)
         sops.sem_flg |= SEM_UNDO;
 
+    //qDebug("QWSLock::up(): %p, semop(%d, %d)", this, (int)semId, (int)semNum);
     EINTR_LOOP(ret, semop(semId, &sops, 1));
 #else
+    //qDebug("QWSLock::up(): %p, sem_post(%d)", this, (int)(sems[semNum]));
     ret = sem_post(sems[semNum]);
 #endif
     if (ret == -1) {
@@ -195,6 +203,7 @@ bool QWSLock::down(unsigned short semNum, int)
     if (semNum == BackingStore)
         sops.sem_flg |= SEM_UNDO;
 
+    //qDebug("QWSLock::down(): %p, semop(%d, %d)", this, (int)semId, (int)semNum);
     EINTR_LOOP(ret, semop(semId, &sops, 1));
 #else
     EINTR_LOOP(ret, sem_wait(sems[semNum]));
diff --git a/src/gui/embedded/qwslock_p.h b/src/gui/embedded/qwslock_p.h
index d324e4f..d867d20 100644
--- a/src/gui/embedded/qwslock_p.h
+++ b/src/gui/embedded/qwslock_p.h
@@ -86,8 +86,8 @@ private:
     int lockCount[2];
 #ifdef QT_POSIX_IPC
     sem_t *sems[3];
-    bool owned;
 #endif
+    bool owned;
 };
 
 QT_END_NAMESPACE
diff --git a/src/gui/kernel/qapplication_qws.cpp b/src/gui/kernel/qapplication_qws.cpp
index 5314777..38ff052 100644
--- a/src/gui/kernel/qapplication_qws.cpp
+++ b/src/gui/kernel/qapplication_qws.cpp
@@ -531,6 +531,7 @@ QWSDisplay::Data::~Data()
         csocket->flush(); // may be pending QCop message, eg.
         delete csocket;
     }
+    //qDebug("QWSDisplay::Data::~Data()");
     delete clientLock;
     clientLock = 0;
 #endif
@@ -722,6 +723,7 @@ void QWSDisplay::Data::reinit( const QString& newAppName )
     mouseFilter = 0;
 
     qt_desktopWidget = 0;
+    //qDebug("QWSDisplay::Data::reinit()");
     delete QWSDisplay::Data::clientLock;
     QWSDisplay::Data::clientLock = 0;
 
diff --git a/src/gui/painting/qwindowsurface_qws.cpp b/src/gui/painting/qwindowsurface_qws.cpp
index 5dbe1c5..c6802a0 100644
--- a/src/gui/painting/qwindowsurface_qws.cpp
+++ b/src/gui/painting/qwindowsurface_qws.cpp
@@ -858,7 +858,9 @@ void QWSMemorySurface::setLock(int lockId)
         return;
     if (memlock != QWSDisplay::Data::getClientLock())
         delete memlock;
+    //qDebug("QWSMemorySurface::setLock(%d)", lockId);
     memlock = (lockId == -1 ? 0 : new QWSLock(lockId));
+    //qDebug("=> %p", memlock);
 }
 #endif // QT_NO_QWS_MULTIPROCESS
 
-- 
1.7.10.4


--=-=-=--