[Gnash-commit] /srv/bzr/gnash/trunk r9714: Introduce SharedObjectLibrary, cleanup code (still using SOL)

[Sandro Santilli]

------------------------------------------------------------
revno: 9714
committer: Sandro Santilli <address@hidden>
branch nick: trunk
timestamp: Wed 2008-09-10 15:34:17 +0200
message:
  Introduce SharedObjectLibrary, cleanup code (still using SOL)
modified:
  libcore/asobj/SharedObject.cpp
  libcore/asobj/SharedObject.h
  libcore/vm/VM.cpp
  libcore/vm/VM.h
  testsuite/actionscript.all/SharedObject.as
    ------------------------------------------------------------
    revno: 9712.1.4
    committer: Sandro Santilli <address@hidden>
    branch nick: mybranch
    timestamp: Wed 2008-09-10 15:32:57 +0200
    message:
      Introduce a SharedObjectLibrary class, managed by the VM keeps track
      of which SharedObject files are already in memory and avoids re-parsing
      them. Cleanup SharedObject code, update expected results.
    modified:
      libcore/asobj/SharedObject.cpp
      libcore/asobj/SharedObject.h
      libcore/vm/VM.cpp
      libcore/vm/VM.h
      testsuite/actionscript.all/SharedObject.as

=== modified file 'libcore/asobj/SharedObject.cpp'
--- a/libcore/asobj/SharedObject.cpp    2008-09-09 22:48:49 +0000
+++ b/libcore/asobj/SharedObject.cpp    2008-09-10 13:32:57 +0000
@@ -46,7 +46,7 @@
 #include "rc.h" // for use of rcfile
 
 namespace {
-gnash::LogFile& dbglogfile = gnash::LogFile::getDefaultInstance();
+//gnash::LogFile& dbglogfile = gnash::LogFile::getDefaultInstance();
 gnash::RcInitFile& rcfile = gnash::RcInitFile::getDefaultInstance();
 }
 
@@ -199,147 +199,32 @@
         return _sol.size(); // TODO: fix this, is bogus
     }
 
+    bool readSOL(const std::string& filename);
+
 private:
 
     SOL _sol;
 };
 
-bool
-SharedObject::flush() const
-{
-    const std::string& filespec = _sol.getFilespec();
-
-#ifdef USE_SOL_READONLY
-    log_debug(_("SharedObject %s not flushed (compiled as read-only mode)"), 
filespec);
-    return false;
-#endif
-
-//    log_debug("Flushing to file %s", filespec);
-
-    VM& vm = getVM();
-
-    if (rcfile.getSOLReadOnly() ) {
-        log_security("Attempting to write object %s when it's SOL Read Only is 
set! Refusing...",
-                     filespec);
-        return false;
-    }
-    
-    // TODO: cache the dataKey in SharedObject prototype on first use ?
-    //       a SharedObject::getDataKey() might do...
-    string_table::key dataKey = vm.getStringTable().find("data");
-    
-    as_value as = const_cast<SharedObject*>(this)->getMember(dataKey);
-    log_debug("data member of this SharedObject is %s", as);
-    boost::intrusive_ptr<as_object> ptr = as.to_object();
-    if ( ! ptr ) {
-        log_aserror("'data' member of SharedObject is not an object (%s)",
-                  as);
-        return true;
-    }
-
-    SOL sol;
-    PropsSerializer props(sol, vm);
-    ptr->visitPropertyValues(props);
-    // We only want to access files in this directory
-    bool ret = sol.writeFile(filespec, getObjectName().c_str());
-    if ( ! ret )
-    {
-        log_error("writing SharedObject file to %s", filespec);
-        return false;
-    }
-
-    log_security("SharedObject '%s' written to filesystem.", filespec);
-    return true;
-}
-
-
-as_value
-sharedobject_clear(const fn_call& fn)
-{
-//    GNASH_REPORT_FUNCTION;
-    boost::intrusive_ptr<SharedObject> obj = 
ensureType<SharedObject>(fn.this_ptr);
-    UNUSED(obj);
-    
-    LOG_ONCE(log_unimpl (__FUNCTION__));
-
-    return as_value();
-}
-
-as_value
-sharedobject_flush(const fn_call& fn)
-{
-//    GNASH_REPORT_FUNCTION;
-    
-    boost::intrusive_ptr<SharedObject> obj = 
ensureType<SharedObject>(fn.this_ptr);
-
-    IF_VERBOSE_ASCODING_ERRORS(
-    if ( fn.nargs )
-    {
-        std::stringstream ss;
-        fn.dump_args(ss);
-        log_aserror(_("Arguments to SharedObject.flush(%s) will be ignored"), 
ss.str());
-    }
-    )
-
-    return as_value(obj->flush());
-}
-
-// Set the file name
-as_value
-sharedobject_getlocal(const fn_call& fn)
-{
-//    GNASH_REPORT_FUNCTION;
-    // This should return a SharedObject, and it's a static function
-    
-    // FIXME:
-    //    We shouldn't have a single SharedObject static, but one for
-    //    each key the user code asks for. These might be maintained
-    //    in a static "library" member of the SharedObject class
-    //    exposing methods to get one element by name, which would be
-    //    either created or reused, taking care about registering
-    //    the static with the VM. There will likely be a need to
-    //    be signaled restarts as unflushed objects should not be
-    //    accessible with their state on restart..
-    //
-    static boost::intrusive_ptr<SharedObject> obj;
-    if ( ! obj )
-    { 
-        obj = new SharedObject();
-        obj->getVM().addStatic(obj.get());
-    }
-    
-    std::string::size_type pos;
-    std::string rootdir;
-    if (fn.nargs > 0) {
-        std::string filespec = fn.arg(0).to_string();
-        // If there is a second argument to getLocal(), it replaces
-        // the default path, which is the swf file name, with this
-        // supplied path.
-        // the object name appears to be the same as the file name, but
-        // minus the suffix. 
-        if ((pos = filespec.find(".sol", 0) == std::string::npos)) {
-            obj->setObjectName(filespec);
-            filespec += ".sol";
-        } else {
-            std::string objname = filespec.substr(0, filespec.size() - 4);
-            obj->setObjectName(objname);
-        }
-        obj->setFilespec(filespec);
-    }
-
-    std::string newspec = rcfile.getSOLSafeDir();
-    if (newspec.size() == 0) {
-        newspec = "/tmp/";
-    }
-
-    // TODO: check if the base dir exists here, or skip the flush
+SharedObjectLibrary::SharedObjectLibrary(VM& vm)
+    :
+    _vm(vm),
+    _soLib()
+{
+    _solSafeDir = rcfile.getSOLSafeDir();
+    if (_solSafeDir.empty()) {
+        log_debug("Empty SOLSafeDir directive: we'll use '/tmp'");
+        _solSafeDir = "/tmp/";
+    }
+
+    // Check if the base dir exists here
     struct stat statbuf;
-    if ( -1 == stat(newspec.c_str(), &statbuf) )
+    if ( -1 == stat(_solSafeDir.c_str(), &statbuf) )
     {
-       log_error("Invalid SOL safe dir %s: %s", newspec, std::strerror(errno));
-        return as_value(false);
+       log_error("Invalid SOL safe dir %s: %s. Won't save any SharedObject.", 
_solSafeDir, std::strerror(errno));
+        _solSafeDir.clear();
     }
-    
+
     // Which URL we should use here is under research.
     // The reference player uses the URL from which definition
     // of the call to SharedObject.getLocal was parsed.
@@ -358,62 +243,47 @@
     // by the 'base' attribute of OBJECT or EMBED tags trough
     // -P base=xxx
     //
-    const std::string& origURL = obj->getVM().getSWFUrl(); 
-    
-    URL url(origURL);
-//  log_debug(_("BASE URL=%s (%s)"), url.str(), url.hostname());
+    const std::string& swfURL = _vm.getSWFUrl();
 
     // Get the domain part, or take as 'localhost' if none
     // (loaded from filesystem)
-    //
-    std::string domain = url.hostname();
-    if (domain.empty()) domain = "localhost";
-    
+    URL url(swfURL);
+//  log_debug(_("BASE URL=%s (%s)"), url.str(), url.hostname());
+    _baseDomain = url.hostname();
+    if ( _baseDomain.empty() ) _baseDomain = "localhost";
+
     // Get the path part
-    std::string swfile = url.path();
+    _basePath = url.path();
+
        // TODO: if the original url was a relative one, the pp uses just
        // the relative portion rather then the resolved absolute path !
-    
-    if ( rcfile.getSOLLocalDomain() && domain != "localhost") 
-    {
-        log_security("Attempting to open non localhost created SOL file!! %s",
-                     obj->getFilespec());
-        return as_value(false);
-    }
-
-    // The optional second argument drops the domain and the swf file name
-    //
-    // NOTE: having more then 2 args should still use the second
-    //       (and discard the subsequents).
-    //
-    if (fn.nargs > 1) {
-        rootdir = fn.arg(1).to_string();
-        log_debug("The rootdir is: %s", rootdir);
-        newspec += rootdir;
-    } else {
-        newspec += "/";    
-        newspec += domain;
-        newspec += "/";
-        newspec += swfile;
-        newspec += "/";
-    }
-    
-    //log_debug("newspec before adding obj's filespec: %s", newspec);
-    newspec += obj->getFilespec();
-    obj->setFilespec(newspec);
-        
-    if (newspec.find("/", 0) != std::string::npos) {
+}
+
+void
+SharedObjectLibrary::markReachableResources() const
+{
+    for (SoLib::const_iterator it=_soLib.begin(), itE=_soLib.end(); it!=itE; 
++it)
+    {
+        SharedObject* sh = it->second;
+        sh->setReachable();
+    }
+}
+
+static bool createDirForFile(const std::string& filename)
+{
+    if (filename.find("/", 0) != std::string::npos)
+    {
         typedef boost::tokenizer<boost::char_separator<char> > Tok;
         boost::char_separator<char> sep("/");
-        Tok t(newspec, sep);
+        Tok t(filename, sep);
         Tok::iterator tit;
         std::string newdir = "/";
         for(tit=t.begin(); tit!=t.end();++tit){
             //cout << *tit << "\n";
             newdir += *tit;
             if (newdir.find("..", 0) != std::string::npos) {
-               log_error("Invalid SharedObject path (contains '..'): %s", 
newspec);
-                return as_value(false);
+               log_error("Invalid SharedObject path (contains '..'): %s", 
filename);
+                return false;
             }
             // Don't try to create a directory of the .sol file name!
             // TODO: don't fail if the movie url has a component ending with 
.sol (eh...)
@@ -427,27 +297,91 @@
                 if ((errno != EEXIST) && (ret != 0)) {
                     log_error("Couldn't create directory for .sol files: 
%s\n\t%s",
                               newdir, std::strerror(errno));
-                    return as_value(false);
+                    return false;
                 }
             } // else log_debug("newdir %s ends with .sol", newdir);
             newdir += "/";
         }
-    } // else log_debug("no slash in filespec %s", obj->getFilespec());
-
+    }
+    else log_debug("no slash in filespec %s", filename);
+    return true;
+}
+
+SharedObject*
+SharedObjectLibrary::getLocal(const std::string& objName, const std::string& 
root)
+{
+    assert ( ! objName.empty() );
+
+    if ( _solSafeDir.empty() ) return 0; // already warned about it at 
construction time
+
+    // TODO: this check sounds kind of lame, fix it
+    if ( rcfile.getSOLLocalDomain() && _baseDomain != "localhost") 
+    {
+        log_security("Attempting to open SOL file from non localhost-loaded 
SWF");
+        return 0;
+    }
+
+    // The optional second argument drops the domain and the swf file name
+    std::string key;
+    if ( root.empty() ) key = "/" + _baseDomain + "/" + _basePath + "/" + 
objName;
+    else key = root + "/" + objName;
+
+    // TODO: normalize key!
+
+    // If the shared object was already opened, use it.
+    SoLib::iterator it = _soLib.find(key);
+    if ( it != _soLib.end() )
+    {
+        log_debug("SharedObject %s already known, returning it", key);
+        return it->second;
+    }
+    log_debug("SharedObject %s not known, creating it", key);
+
+    // Otherwise create a new one and register to the lib
+    SharedObject* obj = new SharedObject();
+    _soLib[key] = obj;
+
+    obj->setObjectName(objName);
+
+    std::string newspec = _solSafeDir;
+    newspec += "/";
+    newspec += key;
+    newspec += ".sol";
+    obj->setFilespec(newspec);
+
+    log_debug("SharedObject path: %s", newspec);
+        
+    if ( ! createDirForFile(newspec) )
+    {
+        log_error("Couldn't create dir for SharedObject %s", newspec);
+        return 0;
+    }
+
+    if ( ! obj->readSOL(newspec) )
+    {
+        log_error("Couldn't read SOL %s, will create on flush/exit", newspec);
+    }
+
+    return obj;
+}
+
+bool
+SharedObject::readSOL(const std::string& newspec)
+{
     SOL sol;
     log_security("Opening SharedObject file: %s", newspec);
     if (sol.readFile(newspec) == false) {
         log_security("empty or non-existing SOL file \"%s\", will be created 
on flush/exit", newspec);
-        return as_value(obj.get());
+        return false;
     }
     
     std::vector<Element *>::const_iterator it, e;
     std::vector<Element *> els = sol.getElements();
     log_debug("Read %d AMF objects from %s", els.size(), newspec);
 
-    string_table& st = obj->getVM().getStringTable();
-    string_table::key dataKey =  obj->getVM().getStringTable().find("data");
-    as_value as = obj->getMember(dataKey);
+    string_table& st = _vm.getStringTable();
+    string_table::key dataKey =  st.find("data");
+    as_value as = getMember(dataKey);
     boost::intrusive_ptr<as_object> ptr = as.to_object();
     
     for (it = els.begin(), e = els.end(); it != e; it++) {
@@ -481,20 +415,137 @@
 
             case Element::OBJECT_AMF0:
                 // TODO: implement!
+                log_unimpl("Reading OBJECT type from SharedObject");
                 //data.convert_to_object();
                 //ptr->set_member(st.string_table::find(el->name), data);
+                return false;
                 break;
 
             default:
                 // TODO: what about other types?
+                log_unimpl("Reading SOL type %d", el->getType());
+                return false;
                 break;
         } 
-
-    }
-
-//    ptr->dump_members();        // FIXME: debug crap
-    
-    return as_value(obj.get()); // will keep alive
+    }
+
+    return true;
+}
+
+bool
+SharedObject::flush() const
+{
+    const std::string& filespec = _sol.getFilespec();
+
+#ifdef USE_SOL_READONLY
+    log_debug(_("SharedObject %s not flushed (compiled as read-only mode)"), 
filespec);
+    return false;
+#endif
+
+//    log_debug("Flushing to file %s", filespec);
+
+    VM& vm = getVM();
+
+    if (rcfile.getSOLReadOnly() ) {
+        log_security("Attempting to write object %s when it's SOL Read Only is 
set! Refusing...",
+                     filespec);
+        return false;
+    }
+    
+    // TODO: cache the dataKey in SharedObject prototype on first use ?
+    //       a SharedObject::getDataKey() might do...
+    string_table::key dataKey = vm.getStringTable().find("data");
+    
+    as_value as = const_cast<SharedObject*>(this)->getMember(dataKey);
+    log_debug("data member of this SharedObject is %s", as);
+    boost::intrusive_ptr<as_object> ptr = as.to_object();
+    if ( ! ptr ) {
+        log_aserror("'data' member of SharedObject is not an object (%s)",
+                  as);
+        return true;
+    }
+
+    SOL sol;
+    PropsSerializer props(sol, vm);
+    ptr->visitPropertyValues(props);
+    // We only want to access files in this directory
+    bool ret = sol.writeFile(filespec, getObjectName().c_str());
+    if ( ! ret )
+    {
+        log_error("writing SharedObject file to %s", filespec);
+        return false;
+    }
+
+    log_security("SharedObject '%s' written to filesystem.", filespec);
+    return true;
+}
+
+
+as_value
+sharedobject_clear(const fn_call& fn)
+{
+//    GNASH_REPORT_FUNCTION;
+    boost::intrusive_ptr<SharedObject> obj = 
ensureType<SharedObject>(fn.this_ptr);
+    UNUSED(obj);
+    
+    LOG_ONCE(log_unimpl (__FUNCTION__));
+
+    return as_value();
+}
+
+as_value
+sharedobject_flush(const fn_call& fn)
+{
+//    GNASH_REPORT_FUNCTION;
+    
+    boost::intrusive_ptr<SharedObject> obj = 
ensureType<SharedObject>(fn.this_ptr);
+
+    IF_VERBOSE_ASCODING_ERRORS(
+    if ( fn.nargs )
+    {
+        std::stringstream ss;
+        fn.dump_args(ss);
+        log_aserror(_("Arguments to SharedObject.flush(%s) will be ignored"), 
ss.str());
+    }
+    )
+
+    return as_value(obj->flush());
+}
+
+// Set the file name
+as_value
+sharedobject_getlocal(const fn_call& fn)
+{
+//    GNASH_REPORT_FUNCTION;
+
+    VM& vm = fn.env().getVM();
+    int swfVersion = vm.getSWFVersion();
+
+    as_value objNameVal;
+    if (fn.nargs > 0) objNameVal = fn.arg(0);
+    std::string objName = objNameVal.to_string_versioned(swfVersion);
+    if ( objName.empty() )
+    {
+        IF_VERBOSE_ASCODING_ERRORS(
+        std::stringstream ss; fn.dump_args(ss);
+        log_aserror("SharedObject.getLocal(%s): %s", _("missing object name"));
+        );
+        as_value ret; ret.set_null();
+        return ret;
+    }
+
+    std::string root;
+    if (fn.nargs > 1)
+    {
+        root = fn.arg(1).to_string_versioned(swfVersion);
+    }
+
+    log_debug("SO name:%s, root:%s", objName, root);
+
+    SharedObject* obj = vm.getSharedObjectLibrary().getLocal(objName, root);
+    as_value ret(obj);
+    log_debug("SharedObject.getLocal returning %s", ret);
+    return ret;
 }
 
 as_value

=== modified file 'libcore/asobj/SharedObject.h'
--- a/libcore/asobj/SharedObject.h      2008-03-26 21:34:20 +0000
+++ b/libcore/asobj/SharedObject.h      2008-09-10 13:32:57 +0000
@@ -20,10 +20,49 @@
 #define __GNASH_ASOBJ_SHAREDOBJECT_H__
 
 #include <memory> // for auto_ptr
-
-namespace gnash {
-
-class as_object;
+#include <string>
+#include <map>
+
+// Forward declarations
+namespace gnash {
+    class as_object;
+    class SharedObject;
+    class VM;
+}
+
+namespace gnash {
+
+class SharedObjectLibrary
+{
+public:
+
+    SharedObjectLibrary(VM& vm);
+
+    /// Return a local shared object with given name and with given root
+    //
+    /// May return NULL if name is invalid or can't access the given root
+    ///
+    SharedObject* getLocal(const std::string& name, const std::string& root);
+
+    void markReachableResources() const;
+
+private:
+
+    VM& _vm;
+
+    /// Domain component of the VM SWF url
+    std::string _baseDomain;
+
+    /// Path component of the VM SWF url
+    std::string _basePath;
+
+    /// Base SOL dir
+    std::string _solSafeDir; 
+
+    typedef std::map<std::string, SharedObject*> SoLib;
+
+    SoLib _soLib;
+};
 
 /// Initialize the global SharedObject class
 void sharedobject_class_init(as_object& global);

=== modified file 'libcore/vm/VM.cpp'
--- a/libcore/vm/VM.cpp 2008-09-04 08:32:58 +0000
+++ b/libcore/vm/VM.cpp 2008-09-10 13:32:57 +0000
@@ -22,6 +22,7 @@
 #endif
 
 #include "VM.h"
+#include "SharedObject.h" // for SharedObjectLibrary
 #include "smart_ptr.h" // GNASH_USE_GC
 #include "builtin_function.h"
 #include "movie_definition.h"
@@ -89,7 +90,8 @@
        _swfurl(topmovie.get_url()),
        mMachine(0),
        _clock(clock),
-       _stack()
+       _stack(),
+    _shLib(new SharedObjectLibrary(*this))
 {
        _clock.restart();
        assert(!_swfurl.empty());
@@ -97,9 +99,7 @@
 
 VM::~VM()
 {
-       // nothing to do atm, but we'll likely
-       // have to deregister lots of stuff when
-       // things are setup
+    delete _shLib;
 }
 
 std::locale&
@@ -242,6 +242,8 @@
        }
 
        mClassHierarchy->markReachableResources();
+
+    _shLib->markReachableResources();
 #endif
 
 }

=== modified file 'libcore/vm/VM.h'
--- a/libcore/vm/VM.h   2008-09-04 08:32:58 +0000
+++ b/libcore/vm/VM.h   2008-09-10 13:32:57 +0000
@@ -37,6 +37,7 @@
 namespace gnash {
        class movie_definition;
        class builtin_function;
+    class SharedObjectLibrary;
        class as_object;
        class Machine;
        class VirtualClock;
@@ -158,6 +159,9 @@
 
        SafeStack<as_value>     _stack;
 
+       /// Library of SharedObjects. Owned by the VM.
+       SharedObjectLibrary* _shLib;
+
 public:
 
        SafeStack<as_value>& getStack()
@@ -264,6 +268,10 @@
        /// Get a pointer to this VM's Root movie (stage)
        movie_root& getRoot() const;
 
+    SharedObjectLibrary& getSharedObjectLibrary() const {
+        return *_shLib;
+    }
+
        /// Get a pointer to this VM's _global Object
        as_object* getGlobal() const;
 

=== modified file 'testsuite/actionscript.all/SharedObject.as'
--- a/testsuite/actionscript.all/SharedObject.as        2008-09-10 11:09:16 
+0000
+++ b/testsuite/actionscript.all/SharedObject.as        2008-09-10 13:32:57 
+0000
@@ -87,7 +87,7 @@
 // But a getLocal call using a *different* "id" returns
 // a different SharedObject...
 so3 = SharedObject.getLocal("level1/level2/settings3", "/");
-xcheck(so3 != so);
+check(so3 != so);
 
 
 // trace(so.getSize());
@@ -134,7 +134,7 @@
 }
 
 so4 = SharedObject.getLocal("Another one", "/subdir");
-xcheck(so4 != so3);
+check(so4 != so3);
 xcheck_equals(typeof(so4.data), 'undefined');
 ret = so4.flush();
 xcheck_equals(typeof(ret), 'undefined');
@@ -164,7 +164,7 @@
 so6 = SharedObject.getLocal("so6");
 a = new Array;
 for (var i in so6) a.push(i);
-xcheck_equals(a.toString(), 'data');
+check_equals(a.toString(), 'data');
 delete so6;
 check_equals(typeof(so.data), 'object');
 so6.data = 5;
@@ -180,16 +180,16 @@
  check(so7 instanceof SharedObject);
 #else
  // returns undefined
- xcheck_equals(typeof(so7), 'null');
+ check_equals(typeof(so7), 'null');
 #endif
 so7.data.a = 1;
 so7.flush();
 
 so8 = SharedObject.getLocal('');
-xcheck_equals(typeof(so8), 'null');
+check_equals(typeof(so8), 'null');
 
 so9 = SharedObject.getLocal('', 'something');
-xcheck_equals(typeof(so9), 'null');
+check_equals(typeof(so9), 'null');
 
 //------------------------------------------
 // END OF TESTS