1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183
|
Description: revert "QProcessEnvironment: simplify locking"
This reverts commit c5d6b263c204cb09db2be36826e19acb03dc24fb.
.
The commit being reverted assumes the mutex is only protecting 'nameMap'
and nothing else is mutable, which is false. The mutex is not only
protecting 'nameMap' but also protecting the containing value objects,
since even though the value object is accessed read-only, its
implementation mutates its internal states for 2-way conversion between
ByteArray and QString.
.
Commit 85e61297f7b02297641826332dbdbc845a88c34b ("restore
QProcessEnvironment shared data thread safety on unix") said that
implicit sharing together with 'mutable' is a time bomb and the bomb is
triggered by the reverted commit.
Origin: upstream, https://code.qt.io/cgit/qt/qtbase.git/commit?id=080d61c020678b75
Last-Update: 2026-01-29
--- a/src/corelib/io/qprocess.cpp
+++ b/src/corelib/io/qprocess.cpp
@@ -202,7 +202,6 @@ void QProcessEnvironmentPrivate::insert(
vars.insert(it.key(), it.value());
#ifdef Q_OS_UNIX
- const OrderedNameMapMutexLocker locker(this, &other);
auto nit = other.nameMap.constBegin();
const auto nend = other.nameMap.constEnd();
for ( ; nit != nend; ++nit)
@@ -276,6 +275,7 @@ bool QProcessEnvironment::operator==(con
return true;
if (d) {
if (other.d) {
+ QProcessEnvironmentPrivate::OrderedMutexLocker locker(d, other.d);
return d->vars == other.d->vars;
} else {
return isEmpty();
@@ -322,6 +322,7 @@ bool QProcessEnvironment::contains(const
{
if (!d)
return false;
+ QProcessEnvironmentPrivate::MutexLocker locker(d);
return d->vars.contains(d->prepareName(name));
}
@@ -372,6 +373,7 @@ QString QProcessEnvironment::value(const
if (!d)
return defaultValue;
+ QProcessEnvironmentPrivate::MutexLocker locker(d);
const auto it = d->vars.constFind(d->prepareName(name));
if (it == d->vars.constEnd())
return defaultValue;
@@ -396,6 +398,7 @@ QStringList QProcessEnvironment::toStrin
{
if (!d)
return QStringList();
+ QProcessEnvironmentPrivate::MutexLocker locker(d);
return d->toList();
}
@@ -409,6 +412,7 @@ QStringList QProcessEnvironment::keys()
{
if (!d)
return QStringList();
+ QProcessEnvironmentPrivate::MutexLocker locker(d);
return d->keys();
}
@@ -425,6 +429,7 @@ void QProcessEnvironment::insert(const Q
return;
// our re-impl of detach() detaches from null
+ QProcessEnvironmentPrivate::MutexLocker locker(e.d);
d->insert(*e.d);
}
--- a/src/corelib/io/qprocess_p.h
+++ b/src/corelib/io/qprocess_p.h
@@ -146,22 +146,16 @@ public:
inline QString nameToString(const Key &name) const { return name; }
inline Value prepareValue(const QString &value) const { return value; }
inline QString valueToString(const Value &value) const { return value; }
-#else
- struct NameMapMutexLocker : public QMutexLocker
- {
- NameMapMutexLocker(const QProcessEnvironmentPrivate *d) : QMutexLocker(&d->nameMapMutex) {}
+ struct MutexLocker {
+ MutexLocker(const QProcessEnvironmentPrivate *) {}
};
- struct OrderedNameMapMutexLocker : public QOrderedMutexLocker
- {
- OrderedNameMapMutexLocker(const QProcessEnvironmentPrivate *d1,
- const QProcessEnvironmentPrivate *d2)
- : QOrderedMutexLocker(&d1->nameMapMutex, &d2->nameMapMutex)
- {}
+ struct OrderedMutexLocker {
+ OrderedMutexLocker(const QProcessEnvironmentPrivate *,
+ const QProcessEnvironmentPrivate *) {}
};
-
+#else
inline Key prepareName(const QString &name) const
{
- const NameMapMutexLocker locker(this);
Key &ent = nameMap[name];
if (ent.isEmpty())
ent = name.toLocal8Bit();
@@ -170,27 +164,40 @@ public:
inline QString nameToString(const Key &name) const
{
const QString sname = QString::fromLocal8Bit(name);
- {
- const NameMapMutexLocker locker(this);
- nameMap[sname] = name;
- }
+ nameMap[sname] = name;
return sname;
}
inline Value prepareValue(const QString &value) const { return Value(value); }
inline QString valueToString(const Value &value) const { return value.string(); }
+ struct MutexLocker : public QMutexLocker
+ {
+ MutexLocker(const QProcessEnvironmentPrivate *d) : QMutexLocker(&d->mutex) {}
+ };
+ struct OrderedMutexLocker : public QOrderedMutexLocker
+ {
+ OrderedMutexLocker(const QProcessEnvironmentPrivate *d1,
+ const QProcessEnvironmentPrivate *d2) :
+ QOrderedMutexLocker(&d1->mutex, &d2->mutex)
+ {}
+ };
+
QProcessEnvironmentPrivate() : QSharedData() {}
QProcessEnvironmentPrivate(const QProcessEnvironmentPrivate &other) :
- QSharedData(), vars(other.vars)
+ QSharedData()
{
+ // This being locked ensures that the functions that only assign
+ // d pointers don't need explicit locking.
// We don't need to lock our own mutex, as this object is new and
// consequently not shared. For the same reason, non-const methods
// do not need a lock, as they detach objects (however, we need to
// ensure that they really detach before using prepareName()).
- NameMapMutexLocker locker(&other);
+ MutexLocker locker(&other);
+ vars = other.vars;
nameMap = other.nameMap;
- // We need to detach our nameMap, so that our mutex can protect it.
- // As we are being detached, it likely would be detached a moment later anyway.
+ // We need to detach our members, so that our mutex can protect them.
+ // As we are being detached, they likely would be detached a moment later anyway.
+ vars.detach();
nameMap.detach();
}
#endif
@@ -201,7 +208,8 @@ public:
#ifdef Q_OS_UNIX
typedef QHash<QString, Key> NameHash;
mutable NameHash nameMap;
- mutable QMutex nameMapMutex;
+
+ mutable QMutex mutex;
#endif
static QProcessEnvironment fromList(const QStringList &list);
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -439,6 +439,7 @@ void QProcessPrivate::startProcess()
int envc = 0;
char **envp = nullptr;
if (environment.d.constData()) {
+ QProcessEnvironmentPrivate::MutexLocker locker(environment.d);
envp = _q_dupEnvironment(environment.d.constData()->vars, &envc);
}
@@ -980,6 +981,7 @@ bool QProcessPrivate::startDetached(qint
int envc = 0;
char **envp = nullptr;
if (environment.d.constData()) {
+ QProcessEnvironmentPrivate::MutexLocker locker(environment.d);
envp = _q_dupEnvironment(environment.d.constData()->vars, &envc);
}
|