zorba-coders team mailing list archive

[Chris Hillery <chillery+launchpad@xxxxxxxxx>]

Chris Hillery has proposed merging lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba.

Requested reviews:
  Chris Hillery (ceejatec)
  Matthias Brantner (matthias-brantner)
Related bugs:
  Bug #996084 in Zorba: "crash in Streamable*Item with file module"
  https://bugs.launchpad.net/zorba/+bug/996084

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105016
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105016
Your team Zorba Coders is subscribed to branch lp:zorba.

=== modified file 'src/runtime/base64/base64_impl.cpp'
--- src/runtime/base64/base64_impl.cpp	2012-05-03 12:31:51 +0000
+++ src/runtime/base64/base64_impl.cpp	2012-05-08 01:12:21 +0000
@@ -52,11 +52,9 @@
     else
     {
       // streamable string eventually transcoding
-      GENV_ITEMFACTORY->createStreamableString(
+      GENV_ITEMFACTORY->createSharedStreamableString(
           result,
-          lItem->getStream(),
-          lItem->getStreamReleaser(),
-          lItem->isSeekable());
+          lItem);
     }
   }
   else

=== modified file 'src/store/api/item_factory.h'
--- src/store/api/item_factory.h	2012-05-03 12:31:51 +0000
+++ src/store/api/item_factory.h	2012-05-08 01:12:21 +0000
@@ -97,6 +97,19 @@
       bool seekable = false) = 0;
 
   /**
+   * Create a StreamableStringItem which re-uses the stream from another
+   * Streamable*Item. This will maintain a reference to the original
+   * item to ensure the stream is not cleaned up before we are done with it.
+   *
+   * It only makes sense to use this method if either (a) the dependent item's
+   * stream is seekable and hence re-usable, or (b) you are sure that the
+   * dependent item will not be utilized after this new item is created.
+   */
+  virtual bool createSharedStreamableString(
+      Item_t& result,
+      Item_t& streamble_dependent) = 0;
+
+  /**
    * Specification: [http://www.w3.org/TR/xmlschema-2/#normalizedString]
    * @param value string representation of the value
    */

=== modified file 'src/store/naive/atomic_items.cpp'
--- src/store/naive/atomic_items.cpp	2012-05-04 14:32:28 +0000
+++ src/store/naive/atomic_items.cpp	2012-05-08 01:12:21 +0000
@@ -1673,6 +1673,36 @@
 /*******************************************************************************
   class StreamableStringItem
 ********************************************************************************/
+StreamableStringItem::StreamableStringItem(
+    std::istream& aStream,
+    StreamReleaser streamReleaser,
+    bool seekable) :
+  theIstream(aStream),
+  theIsMaterialized(false),
+  theIsConsumed(false),
+  theIsSeekable(seekable),
+  theStreamReleaser(streamReleaser),
+  theStreamableDependent(nullptr)
+{
+}
+
+StreamableStringItem::StreamableStringItem(
+    store::Item_t& aStreamableDependent) :
+  theIstream(aStreamableDependent->getStream()),
+  theIsMaterialized(false),
+  theIsConsumed(false),
+  theIsSeekable(aStreamableDependent->isSeekable()),
+  theStreamReleaser(nullptr),
+  theStreamableDependent(aStreamableDependent)
+{
+  ZORBA_ASSERT(theStreamableDependent->isStreamable());
+
+  // We copied the dependent item's stream and seekable flag in the initializer
+  // above, but did NOT copy the StreamReleaser. The dependent item maintains
+  // memory ownership of the stream in this way.
+}
+
+
 void StreamableStringItem::appendStringValue(zstring& aBuf) const
 {
   if (!theIsMaterialized) 

=== modified file 'src/store/naive/atomic_items.h'
--- src/store/naive/atomic_items.h	2012-05-03 12:31:51 +0000
+++ src/store/naive/atomic_items.h	2012-05-08 01:12:21 +0000
@@ -875,6 +875,8 @@
 
   StreamReleaser theStreamReleaser;
 
+  store::Item_t theStreamableDependent;
+
 public:
   bool equals(
         store::Item const*,
@@ -922,15 +924,10 @@
   StreamableStringItem(
       std::istream& aStream,
       StreamReleaser streamReleaser,
-      bool seekable = false)
-    :
-    theIstream(aStream),
-    theIsMaterialized(false),
-    theIsConsumed(false),
-    theIsSeekable(seekable),
-    theStreamReleaser(streamReleaser)
-  {
-  }
+      bool seekable = false);
+
+  StreamableStringItem(
+      store::Item_t& aStreamableDependent);
 
   void materialize() const;
 };

=== modified file 'src/store/naive/simple_item_factory.cpp'
--- src/store/naive/simple_item_factory.cpp	2012-05-03 12:31:51 +0000
+++ src/store/naive/simple_item_factory.cpp	2012-05-08 01:12:21 +0000
@@ -157,6 +157,14 @@
   return true;
 }
 
+bool BasicItemFactory::createSharedStreamableString(
+    store::Item_t &result,
+    store::Item_t &streamable_dependent)
+{
+  result = new StreamableStringItem( streamable_dependent );
+  return true;
+}
+
 
 bool BasicItemFactory::createNormalizedString(store::Item_t& result, zstring& value)
 {

=== modified file 'src/store/naive/simple_item_factory.h'
--- src/store/naive/simple_item_factory.h	2012-05-03 12:31:51 +0000
+++ src/store/naive/simple_item_factory.h	2012-05-08 01:12:21 +0000
@@ -106,6 +106,10 @@
       StreamReleaser,
       bool seekable = false);
 
+  bool createSharedStreamableString(
+      store::Item_t& result,
+      store::Item_t& streamable_dependent);
+
   bool createBase64Binary(store::Item_t& result, xs_base64Binary value);
 
   bool createBase64Binary(

=== added file 'test/rbkt/ExpQueryResults/zorba/base64/reuse-stream.xml.res'
--- test/rbkt/ExpQueryResults/zorba/base64/reuse-stream.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/base64/reuse-stream.xml.res	2012-05-08 01:12:21 +0000
@@ -0,0 +1,1 @@
+<file><content>f0VMRgEBAQAAAAAAAAAAAAMAAwABAAAAIPxDADQAAAD0JTMHAAAAADQAIAAHACgAKAAlAAEAAAAAAAAAAAAAAAAAAAAv22gBL9toAQUAAAAAEAAAAQAAAJTbaAGU62gBlOtoAehnBwAwpgcABgAAAAAQAAACAAAAvKBuAbywbgG8sG4BGAEAABgBAAAGAAAABAAAAAQAAAAUAQAAFAEAABQBAAAkAAAAJAAAAAQAAAAEAAAAUOV0ZKyHJwGshycBrIcnAfyBCAD8gQgABAAAAAQAAABR5XRkAAAAAAAAAAAAAAAAAAAAAAAAAAAGAAAABAAAAFLldGSU22gBlOtoAZTraAFs5AUAbOQFAAQAAAABAAAABAAAABQAAAADAAAAR05VANMOFlHRDhyfmkw9H3+lYuAncwGhG0AAAAYDAAAAIAAAEgAAABBkRVIkEIgUIwhRukiBHLCOBAAEAgggoSCCg0CkED5YAAAGFAIgABkACJAAiQAwAFCQEwAQIABBAIIFYAIACAASAAZARACAABkTpAQB5EQIYEF0IBIAIgAhciYEoEICZGgAAQAABCAAkBACZAJAAQQUEhGAIIAAKBULIJYAAABRAKFV4AIWgQAEIYgAAIAAQECAMgBGiCIGiRAEkAABAMAIgEQAAAQAACgIJYAAAEAAAAGAAgtCpFwEAAZRQACAABAICAASiBBFgRhIAkEBMAApJchACQZDAIAAgABIAKQEgKEACGE05FgCAAAgACEAgEAAQoxkUAEAiBAGAAkCFiBoISRFFCBEBMV3gARBgBAosUJBQQAEEDIAUUCBAABAEAhgAEAAAAAAACApACIBBgHKkAWBQAAEAAAAAACDURggAkEEEIlEgIEADQZwAAATC5AQNYxCABABAAAAAAEAlQD0WGD3gAAAIEDKCCAQQMGNIAAQAAAACECCAADAbMAWABABaRAAUFQCgAAAgxJmDAEIAiIZCmIAIAIEgAESCSAjETEQIws2A8AIgAEAAAAAkGkAiEAKDEYEBRGJhFEAIACICAIBKMAGAAAHIAQBqIgAUAECgAAAAhoEER4BASMBBhEIIQGCAFbg8IiBYAgCAQgAgEBwCAAAIAEAQATGAj+QQaAIIQAIAAShAgg2AKQAAAQBkCoASAAAFAAgAYFqAQDCcoOEg0sAIhKAABQAAgpAAghAAIQgIBAkAAQBDA4gRAACAAIgAPBAUgBDARCAggB0GQBaEAg1Mk8CABEAAAQgBBgAECIAvEIhFCRAIBgAAEALyBEQAhACQoIKwAABkBiAVM4QAQlFgBAS4AASAIIkAZEkEgAsBMCABSCAKACAIBMAD4JSLEyAAGQBBqATAUECQUgACQGBGA==</content><md5>f027633a677f42caf1544f6913bc2dab</md5></file>
\ No newline at end of file

=== added file 'test/rbkt/Queries/zorba/base64/reuse-stream.xq'
--- test/rbkt/Queries/zorba/base64/reuse-stream.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/base64/reuse-stream.xq	2012-05-08 01:12:21 +0000
@@ -0,0 +1,16 @@
+(: From bug 996084. We need to ensure that a streamable base64 item can be re-used. :)
+
+import module namespace base64="http://www.zorba-xquery.com/modules/converters/base64";;
+import module namespace hash = "http://www.zorba-xquery.com/modules/cryptography/hash";;
+import module namespace file = "http://expath.org/ns/file";;
+
+declare variable $filename as xs:anyURI := resolve-uri("decoded");
+
+let $data as xs:base64Binary := file:read-binary ($filename)
+let $md5 as xs:string := hash:md5(base64:decode($data))
+
+return 
+  <file>
+    <content>{$data}</content>
+    <md5>{$md5}</md5>
+  </file>