{"id":106864,"date":"2022-07-14T07:00:00","date_gmt":"2022-07-14T14:00:00","guid":{"rendered":"https:\/\/devblogs.microsoft.com\/oldnewthing\/?p=106864"},"modified":"2022-07-14T07:33:07","modified_gmt":"2022-07-14T14:33:07","slug":"20220714-00","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/oldnewthing\/20220714-00\/?p=106864","title":{"rendered":"Processing a ValueSet or PropertySet even in the face of possible mutation, part 3"},"content":{"rendered":"

We’ve looked at a few different ways of one way of saving the contents of a ValueSet or PropertySet while remaining resilient to concurrency modification. A customer tried to implement the first pattern, where each mutation is followed by a processing pass that bails out if it notices a subsequent modification<\/a>.<\/p>\n

A customer tried to implement this pattern, but with a wrinkle: The collection was not self-saving; rather, the collection was part of a larger object that had an explicit Save<\/code> method. They factored the code this way:<\/p>\n

\/\/ Interface declaration\r\nnamespace Contoso\r\n{\r\n    runtimeclass Widget\r\n    {\r\n        Windows.Foundation.Collections.PropertySet ExtendedProperties { get; };\r\n        void Save();\r\n    }\r\n}\r\n\r\n\/\/ C++\/WinRT code-behind\r\nstruct Widget : WidgetT<Widget>\r\n{\r\n    winrt::PropertySet ExtendedProperties() { return m_propertySet; }\r\n    void Save();\r\n\r\n    winrt::PropertySet m_propertySet;\r\n};\r\n<\/pre>\n
The Save<\/code> method followed the “abandon on failure” pattern:<\/p>\n
void Widget::Save()\r\n{\r\n    try {\r\n        SomeKindOfDataBuffer buffer;\r\n        auto it = m_propertySet.First();\r\n        if (it.HasCurrent()) {\r\n            do {\r\n                auto current = it.Current();\r\n                buffer.AddKeyAndValue(current.Key(), current.Value());\r\n            } while (it.MoveNext());\r\n        }\r\n\r\n        auto guard = m_lock.lock();\r\n\r\n        \/\/ verify that the collection is still unchanged before saving\r\n        std::ignore = it.HasCurrent();\r\n\r\n        SaveToFile(buffer);\r\n    } catch (winrt::hresult_changed_state const&) {\r\n        \/\/ Abandon the operation.\r\n        \/\/ The mutating thread will do its own Save.\r\n        return;\r\n    }\r\n}\r\n<\/pre>\n
(Observe that this is a direct copy\/pasta from  our first pattern<\/a>.)<\/p>\n
Was this the correct implementation?<\/p>\n
No.<\/p>\n
In the two patterns discussed so far, the object was auto-saving. Therefore, if there was a conflicting modification, we know that the modifying thread will also perform its own Save()<\/code>, and we could therefore just abandon the work, knowing that the other thread will assume responsibility for saving. (If that other thread subsequently fails due to a conflict modification, then the responsibility transfers to whoever made the conflicting modification. Eventually, the last modification will make it all the way to the end and save the collection for real.)<\/p>\n
But the Widget<\/code> object is not auto-saving. If the Save<\/code> fails due to a concurrent modification, the conflicting thread is not going to “take over” the Save<\/code> operation since there is no Save<\/code> happening on the conflicting thread.<\/p>\n
For this pattern, you have to decide what your object’s policy is if a conflicting modification is made during a Save<\/code>.<\/p>\n
One option you might choose is that Save()<\/code> silently fails if a concurrent modification occurs, under the expectation that the mutating thread will eventually perform its own Save()<\/code> to bring things back in sync. Though you might want to change it so that the Save()<\/code> method reports whether the save was interrupted.<\/p>\n
bool Widget::TrySave()<\/span>\r\n{\r\n    try {\r\n        SomeKindOfDataBuffer buffer;\r\n        auto it = m_propertySet.First();\r\n        if (it.HasCurrent()) {\r\n            do {\r\n                auto current = it.Current();\r\n                buffer.AddKeyAndValue(current.Key(), current.Value());\r\n            } while (it.MoveNext());\r\n        }\r\n\r\n        auto guard = m_lock.lock();\r\n\r\n        \/\/ verify that the collection is still unchanged before saving\r\n        std::ignore = it.HasCurrent();\r\n\r\n        SaveToFile(buffer);\r\n        return true;<\/span>\r\n    } catch (winrt::hresult_changed_state const&) {\r\n        \/\/ Abandon the operation.\r\n        \/\/ The mutating thread will<\/span><\/span>should<\/span> do its own Save.\r\n        return false;<\/span>\r\n    }\r\n}\r\n<\/pre>\n
This model assumes that everybody will want to (eventually) save their changes. Consider this guy:<\/p>\n
    Widget widget;\r\n\r\n    \/\/ Temporarily mark it as busy.\r\n    widget.ExtendedProperties().Insert(L\"IsBusy\", winrt::box_value(true));\r\n\r\n    DoSomething(widget);\r\n\r\n    \/\/ Okay, not busy any more.\r\n    widget.ExtendedProperties().Remove(L\"IsBusy\");\r\n<\/pre>\n
This caller has no intention of saving the changes. But their temporary modification of the collection may have prevent somebody else from saving. Then again, if the timing were different, their temporary modifications would have been saved by mistake! You get to decide if this is a pattern you want to follow.<\/p>\n
Another option is to say make it forbidden to modify the Widget<\/code>‘s properties while it is being saved. In that case, you would just remove all the concurrent modification protection from the Save<\/code> method and let the “changed state” exception propagate to the caller:<\/p>\n
void Widget::Save()\r\n{\r\n    \/\/ try {<\/span><\/span>\r\n        SomeKindOfDataBuffer buffer;\r\n        auto it = m_propertySet.First();\r\n        if (it.HasCurrent()) {\r\n            do {\r\n                auto current = it.Current();\r\n                buffer.AddKeyAndValue(current.Key(), current.Value());\r\n            } while (it.MoveNext());\r\n        }\r\n\r\n        \/\/ auto guard = m_lock.lock();<\/span><\/span>\r\n\r\n        \/\/ verify that the collection is still unchanged before saving<\/span><\/span>\r\n        \/\/ std::ignore = it.HasCurrent();<\/span><\/span>\r\n\r\n        SaveToFile(buffer);\r\n    \/\/ } catch (winrt::hresult_changed_state const&) {<\/span><\/span>\r\n        \/\/ Abandon the operation.<\/span><\/span>\r\n        \/\/ The mutating thread will do its own Save.<\/span><\/span>\r\n        \/\/ return;<\/span><\/span>\r\n    \/\/ }<\/span><\/span>\r\n}\r\n<\/pre>\n
The only reason for manually iterating was so we could make a final HasCurrent()<\/code> check at the end, but since that got deleted, we can return to the simple version:<\/p>\n
void Widget::Save()\r\n{\r\n    SomeKindOfDataBuffer buffer;\r\n    for (auto [name, value] : m_propertySet)\r\n        buffer.AddKeyAndValue(name, value);\r\n    }\r\n\r\n    SaveToFile(buffer);\r\n}\r\n<\/pre>\n
If a concurrent modification occurs, then the Save()<\/code> method fails with a “changed state” exception, which tells the caller, “You broke the rules and modified the collection while it was saving. Shame on you!”<\/p>\n
Note that there is a tiny window where the concurrent modification is not detected, if it happens after we build the buffer and before we save it. That’s not a problem, because as far as the caller can tell, the mutation could very well have occurred at the ret<\/code> instruction at the end of the Save\u00adTo\u00adFile<\/code> function. (The caller has no insight into the moment in time after the buffer is built and the buffer being saved.)<\/p>\n
Yet another pattern is to say that if you mutate the collection during a Save<\/code>, then you can’t predict whether it will save the pre-mutation version or post-mutation version, but will always save something<\/i>. In that case, we need to back out and retry if a concurrent mutation is encountered.<\/p>\n
void Widget::Save()\r\n{\r\n    while (true) {\r\n        try {\r\n            SomeKindOfDataBuffer buffer;\r\n            for (auto [name, value] : m_propertySet)\r\n                buffer.AddKeyAndValue(name, value);\r\n            }\r\n\r\n            SaveToFile(buffer);\r\n\r\n            return;\r\n\r\n        } catch (winrt::hresult_changed_state const&) {\r\n            \/\/ Abandon the operation and try again.\r\n        }\r\n    }\r\n}\r\n<\/pre>\n
The customer also had another scenario where they needed to process a PropertySet in the fact of concurrent mutation. We’ll look at that next time.<\/p>\n","protected":false},"excerpt":{"rendered":"
Applying and adapting the pattern.<\/p>\n","protected":false},"author":1069,"featured_media":111744,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[25],"class_list":["post-106864","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
Applying and adapting the pattern.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/106864","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=106864"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/106864\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/111744"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=106864"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=106864"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=106864"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}