aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar halcanary <halcanary@google.com>2014-06-27 10:37:27 -0700
committerGravatar Commit bot <commit-bot@chromium.org>2014-06-27 10:37:27 -0700
commit1f8ed022226c9f960b9fc95af9297d5111a07ead (patch)
treef87515d7c897addf9e90c1df29a5669b4f086fb0
parente5c1e3cd63e22bb06c24dd051f4d814f24786c08 (diff)
Add lock to SkPDFDict
Add mutex lock to all functions. Remove dictionary iterator, and replace with new thread-safe functions. BUG=skia:2683 R=mtklein@google.com, djsollen@google.com Author: halcanary@google.com Review URL: https://codereview.chromium.org/360473005
-rw-r--r--src/pdf/SkPDFStream.cpp12
-rw-r--r--src/pdf/SkPDFStream.h10
-rw-r--r--src/pdf/SkPDFTypes.cpp89
-rw-r--r--src/pdf/SkPDFTypes.h32
4 files changed, 84 insertions, 59 deletions
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index e570976403..815ef481ee 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/src/pdf/SkPDFStream.cpp
@@ -36,15 +36,9 @@ SkPDFStream::SkPDFStream(const SkPDFStream& pdfStream)
fState = kCompressed_State;
removeLength = false;
}
- SkPDFDict::Iter dict(pdfStream);
- SkPDFName* key;
- SkPDFObject* value;
- SkPDFName lengthName("Length");
- for (key = dict.next(&value); key != NULL; key = dict.next(&value)) {
- if (removeLength && *key == lengthName) {
- continue;
- }
- this->insert(key, value);
+ this->mergeFrom(pdfStream);
+ if (removeLength) {
+ this->remove("Length");
}
}
diff --git a/src/pdf/SkPDFStream.h b/src/pdf/SkPDFStream.h
index 6371bc187d..636ea984e3 100644
--- a/src/pdf/SkPDFStream.h
+++ b/src/pdf/SkPDFStream.h
@@ -35,10 +35,7 @@ public:
explicit SkPDFStream(SkData* data);
/** Deprecated constructor. */
explicit SkPDFStream(SkStream* stream);
- /** Create a PDF stream with the same content and dictionary entries
- * as the passed one.
- */
- explicit SkPDFStream(const SkPDFStream& pdfStream);
+
virtual ~SkPDFStream();
// The SkPDFObject interface.
@@ -54,6 +51,11 @@ protected:
kCompressed_State, //!< The stream's already been compressed.
};
+ /** Create a PDF stream with the same content and dictionary entries
+ * as the passed one.
+ */
+ explicit SkPDFStream(const SkPDFStream& pdfStream);
+
/* Create a PDF stream with no data. The setData method must be called to
* set the data.
*/
diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index e3cc90c1de..7b79e411b6 100644
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -396,14 +396,26 @@ SkPDFDict::~SkPDFDict() {
clear();
}
+int SkPDFDict::size() const {
+ SkAutoMutexAcquire lock(fMutex);
+ return fValue.count();
+}
+
+
void SkPDFDict::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
bool indirect) {
if (indirect) {
return emitIndirectObject(stream, catalog);
}
+ SkAutoMutexAcquire lock(fMutex); // If another thread triggers a
+ // resize while this thread is in
+ // the for-loop, we can be left
+ // with a bad fValue[i] reference.
stream->writeText("<<");
for (int i = 0; i < fValue.count(); i++) {
+ SkASSERT(fValue[i].key);
+ SkASSERT(fValue[i].value);
fValue[i].key->emitObject(stream, catalog, false);
stream->writeText(" ");
fValue[i].value->emit(stream, catalog, false);
@@ -417,69 +429,84 @@ size_t SkPDFDict::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
return getIndirectOutputSize(catalog);
}
+ SkAutoMutexAcquire lock(fMutex); // If another thread triggers a
+ // resize while this thread is in
+ // the for-loop, we can be left
+ // with a bad fValue[i] reference.
size_t result = strlen("<<>>") + (fValue.count() * 2);
for (int i = 0; i < fValue.count(); i++) {
+ SkASSERT(fValue[i].key);
+ SkASSERT(fValue[i].value);
result += fValue[i].key->getOutputSize(catalog, false);
result += fValue[i].value->getOutputSize(catalog, false);
}
return result;
}
-SkPDFObject* SkPDFDict::insert(SkPDFName* key, SkPDFObject* value) {
- key->ref();
- value->ref();
- struct Rec* newEntry = fValue.append();
- newEntry->key = key;
- newEntry->value = value;
+SkPDFObject* SkPDFDict::append(SkPDFName* key, SkPDFObject* value) {
+ SkASSERT(key);
+ SkASSERT(value);
+ SkAutoMutexAcquire lock(fMutex); // If the SkTDArray resizes while
+ // two threads access array, one
+ // is left with a bad pointer.
+ *(fValue.append()) = Rec(key, value);
return value;
}
+SkPDFObject* SkPDFDict::insert(SkPDFName* key, SkPDFObject* value) {
+ return this->append(SkRef(key), SkRef(value));
+}
+
SkPDFObject* SkPDFDict::insert(const char key[], SkPDFObject* value) {
- value->ref();
- struct Rec* newEntry = fValue.append();
- newEntry->key = new SkPDFName(key);
- newEntry->value = value;
- return value;
+ return this->append(new SkPDFName(key), SkRef(value));
}
void SkPDFDict::insertInt(const char key[], int32_t value) {
- struct Rec* newEntry = fValue.append();
- newEntry->key = new SkPDFName(key);
- newEntry->value = new SkPDFInt(value);
+ (void)this->append(new SkPDFName(key), new SkPDFInt(value));
}
void SkPDFDict::insertScalar(const char key[], SkScalar value) {
- struct Rec* newEntry = fValue.append();
- newEntry->key = new SkPDFName(key);
- newEntry->value = new SkPDFScalar(value);
+ (void)this->append(new SkPDFName(key), new SkPDFScalar(value));
}
void SkPDFDict::insertName(const char key[], const char name[]) {
- struct Rec* newEntry = fValue.append();
- newEntry->key = new SkPDFName(key);
- newEntry->value = new SkPDFName(name);
+ (void)this->append(new SkPDFName(key), new SkPDFName(name));
}
void SkPDFDict::clear() {
+ SkAutoMutexAcquire lock(fMutex);
for (int i = 0; i < fValue.count(); i++) {
+ SkASSERT(fValue[i].key);
+ SkASSERT(fValue[i].value);
fValue[i].key->unref();
fValue[i].value->unref();
}
fValue.reset();
}
-SkPDFDict::Iter::Iter(const SkPDFDict& dict)
- : fIter(dict.fValue.begin()),
- fStop(dict.fValue.end()) {
+void SkPDFDict::remove(const char key[]) {
+ SkASSERT(key);
+ SkPDFName name(key);
+ SkAutoMutexAcquire lock(fMutex);
+ for (int i = 0; i < fValue.count(); i++) {
+ SkASSERT(fValue[i].key);
+ if (*(fValue[i].key) == name) {
+ fValue[i].key->unref();
+ SkASSERT(fValue[i].value);
+ fValue[i].value->unref();
+ fValue.removeShuffle(i);
+ return;
+ }
+ }
}
-SkPDFName* SkPDFDict::Iter::next(SkPDFObject** value) {
- if (fIter != fStop) {
- const Rec* cur = fIter;
- fIter++;
- *value = cur->value;
- return cur->key;
+void SkPDFDict::mergeFrom(const SkPDFDict& other) {
+ SkAutoMutexAcquire lockOther(other.fMutex);
+ SkTDArray<Rec> copy(other.fValue);
+ lockOther.release(); // Do not hold both mutexes at once.
+
+ SkAutoMutexAcquire lock(fMutex);
+ for (int i = 0; i < copy.count(); i++) {
+ *(fValue.append()) = Rec(SkRef(copy[i].key), SkRef(copy[i].value));
}
- *value = NULL;
- return NULL;
}
diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h
index 1f06c4c5fe..004c767843 100644
--- a/src/pdf/SkPDFTypes.h
+++ b/src/pdf/SkPDFTypes.h
@@ -369,7 +369,7 @@ public:
/** The size of the dictionary.
*/
- int size() { return fValue.count(); }
+ int size() const;
/** Add the value to the dictionary with the given key. Refs value.
* @param key The key for this dictionary entry.
@@ -424,28 +424,30 @@ public:
*/
void clear();
+protected:
+ /** Use to remove a single key from the dictionary.
+ */
+ void remove(const char key[]);
+
+ /** Insert references to all of the key-value pairs from the other
+ * dictionary into this one.
+ */
+ void mergeFrom(const SkPDFDict& other);
+
private:
struct Rec {
- SkPDFName* key;
- SkPDFObject* value;
+ SkPDFName* key;
+ SkPDFObject* value;
+ Rec(SkPDFName* k, SkPDFObject* v) : key(k), value(v) {}
};
-public:
- class Iter {
- public:
- explicit Iter(const SkPDFDict& dict);
- SkPDFName* next(SkPDFObject** value);
-
- private:
- const Rec* fIter;
- const Rec* fStop;
- };
-
-private:
static const int kMaxLen = 4095;
+ mutable SkMutex fMutex; // protects modifications to fValue
SkTDArray<struct Rec> fValue;
+ SkPDFObject* append(SkPDFName* key, SkPDFObject* value);
+
typedef SkPDFObject INHERITED;
};