Skip to content
This repository
Browse code

Merge pull request #3371 from theuni/android-jholder-rewrite

Android jholder rewrite
  • Loading branch information...
commit 44b79b402990ae62a7f9a78d1d75e4e3eae48d89 2 parents 697ddbc + 703dbd6
Cory Fields authored October 10, 2013
1  xbmc/android/jni/Context.cpp
@@ -63,7 +63,6 @@ CJNIContext::CJNIContext(const ANativeActivity *nativeActivity)
63 63
 CJNIContext::~CJNIContext()
64 64
 {
65 65
   m_appInstance = NULL;
66  
-  m_context.release();
67 66
   xbmc_jni_on_unload();
68 67
 }
69 68
 
8  xbmc/android/jni/jutils/jutils-details.hpp
@@ -94,7 +94,7 @@ struct jcast_helper<std::string, jhstring>
94 94
 {
95 95
     static std::string cast(jhstring const &v)
96 96
     {
97  
-        return jcast_helper<std::string, jstring>::cast(v);
  97
+        return jcast_helper<std::string, jstring>::cast(v.get());
98 98
     }
99 99
 };
100 100
 
@@ -103,7 +103,7 @@ struct jcast_helper<std::vector<std::string>, jhobjectArray>
103 103
 {
104 104
     static std::vector<std::string> cast(jhobjectArray const &v)
105 105
     {
106  
-        return jcast_helper<std::vector<std::string>, jobjectArray>::cast(v);
  106
+        return jcast_helper<std::vector<std::string>, jobjectArray>::cast(v.get());
107 107
     }
108 108
 };
109 109
 
@@ -112,7 +112,7 @@ struct jcast_helper<std::vector<T>, jhobjectArray>
112 112
 {
113 113
     static std::vector<T> cast(jhobjectArray const &v)
114 114
     {
115  
-        return jcast_helper<std::vector<T>, jobjectArray>::cast(v);
  115
+        return jcast_helper<std::vector<T>, jobjectArray>::cast(v.get());
116 116
     }
117 117
 };
118 118
 
@@ -130,7 +130,7 @@ std::vector<T> jcast_helper<std::vector<T>, jobjectArray>::cast(jobjectArray con
130 130
 
131 131
   for (int i = 0; i < size; i++)
132 132
   {
133  
-    jhobject element = (jhobject)env->GetObjectArrayElement(v, i);
  133
+    T element((jhobject)env->GetObjectArrayElement(v, i));
134 134
     vec.emplace_back(element);
135 135
   }
136 136
   return vec;
249  xbmc/android/jni/jutils/jutils.hpp
@@ -57,99 +57,208 @@ JNIEnv *xbmc_jnienv();
57 57
 
58 58
 namespace jni
59 59
 {
  60
+
60 61
 template <typename T>
61 62
 class jholder
62 63
 {
63  
-    typedef void (jholder::*safe_bool_type)();
64  
-    void non_null_object() {}
  64
+/* A native jni types wrapper class.
  65
+
  66
+  This templated class is used as a container for native jni types: jobject,
  67
+  jclass, etc. It maintains scope and references so that parent objects don't
  68
+  have to bother.
  69
+
  70
+  Here, a jobject will be used as an example.
  71
+
  72
+  Background: JNI uses reference-counted objects to facilitate
  73
+  garbage-collection. A jobject is really just a pointer to some shared memory.
  74
+  When a jobject is given to native code, it has a local
  75
+  reference. When this local reference is removed, the JVM is free to
  76
+  garbage-collect its contents. Local references are destroyed when the jobject
  77
+  is destroyed (or loses scope) or when Java execution resumes, or they can be
  78
+  destroyed manually. Objects which hold local references also cannot be shared
  79
+  between threads. To get around these limitations, the local reference can be
  80
+  upgraded to a global one.
  81
+
  82
+  This class handles this logic for the user. A jobject can be moved into a
  83
+  jhobject via the jhobject constructor. After doing so, the original jobject's
  84
+  state is undefined because the jhobject will unref it automatically.
  85
+
  86
+  Scope of copies is handled as well, so no extra care needs to be taken. When
  87
+  a copy is made, the copy inherits the scope of the original object. The
  88
+  class tries to operate in the safest possible way when assigning objects of
  89
+  differing scope by choosing the widest possible scope.
  90
+
  91
+  Example:
  92
+
  93
+  jhobject somefunction(jobject androidObject)
  94
+  {
  95
+    jhobject foo(androidObject);
  96
+    // store androidObject in foo. androidObject should not be used again.
  97
+    // foo has a local ref.
  98
+
  99
+    jhobject bar(foo);
  100
+    // copy foo to bar. foo and bar are both local. They can both be used
  101
+    // on this thread until returning to java. It would not be safe to return
  102
+    // them because the caller cannot be trusted to obey the local-ref rules.
  103
+    //
  104
+    // Note: This copy makes no practical sense, it's only used for
  105
+    // demonstrating scope copies. This is effectively making a copy of a
  106
+    // pointer.
  107
+
  108
+    bar.setGlobal();
  109
+    // Now foo has a local ref and bar has a global ref.
  110
+
  111
+    foo = bar;
  112
+    // foo's local ref is destroyed. it's value is set to bar's and the widest
  113
+    // possible scope is chosen. foo and bar now both have global refs.
  114
+
  115
+    return foo;
  116
+  } // bar's global is unref'd and it is destroyed. global foo is returned.
  117
+
  118
+*/
  119
+
  120
+typedef void (jholder::*safe_bool_type)();
  121
+void non_null_object() {}
65 122
 
66 123
 public:
67  
-    explicit jholder(T obj = 0)
68  
-        :object(obj)
69  
-        ,global(false)
70  
-    {
71  
-    }
72 124
 
73  
-    jholder(jholder const &c)
74  
-        :object(c.object ? (T)xbmc_jnienv()->NewLocalRef(c.object) : 0)
75  
-        ,global(false)
76  
-    {
77  
-    }
  125
+/*! \brief constructor for jholder().
  126
+   Object is null, type is invalid
  127
+*/
  128
+  jholder()
  129
+  :m_refType(JNIInvalidRefType)
  130
+  ,m_object(0)
  131
+  {
  132
+  }
78 133
 
  134
+/*! \brief jholder copy constructor.
  135
+   Object is copied, scope matches the original
  136
+*/
  137
+  jholder(jholder<T> const &c)
  138
+  :m_refType(JNIInvalidRefType)
  139
+  ,m_object(c.get())
  140
+  {
  141
+    setscope(c.m_refType);
  142
+  }
79 143
 
80  
-    template <typename U>
81  
-    explicit jholder(jholder<U> const &c)
82  
-        :object(c.object ? (T)xbmc_jnienv()->NewLocalRef(c.object) : 0)
83  
-        ,global(false)
84  
-    {
85  
-    }
  144
+/*! \brief jholder assignment operator
  145
+   Object takes on the widest scope of the two, local at a minimum
  146
+*/
  147
+  jholder &operator=(jholder const &c)
  148
+  {
  149
+    jobjectRefType newtype = JNILocalRefType;
  150
+    if(c.m_refType == JNIGlobalRefType || m_refType == JNIGlobalRefType)
  151
+      newtype = JNIGlobalRefType;
86 152
 
87  
-    ~jholder()
88  
-    {
89  
-      reset();
90  
-    }
  153
+    reset(c.get());
  154
+    setscope(newtype);
  155
+    return *this;
  156
+  }
91 157
 
92  
-    jholder &operator=(jholder const &c)
93  
-    {
94  
-        jholder tmp(c);
95  
-        swap(tmp);
96  
-        return *this;
97  
-    }
  158
+/*! \brief jholder constructor from a native jni object
  159
+   Incoming objects already hold a local ref.
  160
+*/
  161
+  explicit jholder(const T& obj)
  162
+  :m_refType(JNILocalRefType)
  163
+  ,m_object(obj)
  164
+  {
  165
+    setscope(JNILocalRefType);
  166
+  }
98 167
 
99  
-    void reset(T obj = 0)
100  
-    {
101  
-        if (object)
102  
-        {
103  
-          if(global)
104  
-            xbmc_jnienv()->DeleteGlobalRef(object);
105  
-          else
106  
-            xbmc_jnienv()->DeleteLocalRef(object);
107  
-        }
108  
-        object = obj;
109  
-        global = false;
110  
-    }
  168
+/*! \brief jholder dtor.
  169
+   Held objects are deref'd and destroyed.
  170
+*/
  171
+  ~jholder()
  172
+  {
  173
+    reset();
  174
+  }
111 175
 
112  
-    const T& get() const {return object;}
  176
+/*! \brief cast operator for native types
  177
+   Grr, no explicit operator casts without c++11.
  178
+   This enables automatic down-casting to native types.
  179
+*/
  180
+  operator const T&() const
  181
+  {
  182
+    return get();
  183
+  }
113 184
 
114  
-    T release()
115  
-    {
116  
-        T ret = object;
117  
-        object = 0;
118  
-        global = false;
119  
-        return ret;
120  
-    }
  185
+/*! \brief get native type
  186
+   Same as the above, mainly for internal usage for readability and explicitness
  187
+*/
  188
+  const T& get() const
  189
+  {
  190
+    return m_object;
  191
+  }
121 192
 
122  
-    void setGlobal()
123  
-    {
124  
-      if(object)
125  
-      {
126  
-        T globalRef = (T)xbmc_jnienv()->NewGlobalRef(object);
127  
-        reset(globalRef);
128  
-        global = true;
129  
-      }
130  
-    }
  193
+/*! \brief set an object to global scope. Has no effect if it's already global
  194
+*/
  195
+  void setGlobal()
  196
+  {
  197
+    setscope(JNIGlobalRefType);
  198
+  }
131 199
 
132  
-    bool operator!() const {return !object;}
133  
-    operator safe_bool_type () const
134  
-    {
135  
-        return object ? &jholder::non_null_object : 0;
136  
-    }
  200
+/*! \brief not operator.
  201
+     queries whether the jholder contains a valid object
  202
+*/
  203
+  bool operator!() const {return !m_object;}
  204
+  operator safe_bool_type () const
  205
+  {
  206
+    return m_object ? &jholder::non_null_object : 0;
  207
+  }
137 208
 
138  
-    operator const T&() const
  209
+/*! \brief Change the internal object
  210
+  Unref the original object. The new ref type is invalid.
  211
+
  212
+  This should almost never be used outside of this class. Use it only to
  213
+  maintain static objects that should never be unref'd, such as
  214
+  nativeActivity->clazz.
  215
+  Repeat: Usage of reset() is almost definitely wrong!
  216
+*/
  217
+  void reset(const T &obj = 0)
  218
+  {
  219
+    if(m_object)
139 220
     {
140  
-      return object;
  221
+      if(m_refType == JNIGlobalRefType)
  222
+        xbmc_jnienv()->DeleteGlobalRef(m_object);
  223
+      else if (m_refType == JNILocalRefType)
  224
+        xbmc_jnienv()->DeleteLocalRef(m_object);
141 225
     }
  226
+    m_refType = JNIInvalidRefType;
  227
+    m_object = obj;
  228
+  }
142 229
 
143 230
 private:
144  
-    void swap(jholder &c)
  231
+
  232
+/*! \brief Set an object to local/global scope
  233
+    New refs will be created as needed.
  234
+    If the scope is being set to invalid, its ref will be destroyed as needed.
  235
+*/
  236
+  void setscope(const jobjectRefType type)
  237
+  {
  238
+    // Don't attempt anything on a bad object. Update its status to invalid.
  239
+    if (!m_object)
145 240
     {
146  
-        T tmp = object;
147  
-        object = c.object;
148  
-        c.object = tmp;
  241
+      m_refType = JNIInvalidRefType;
  242
+      return;
149 243
     }
150 244
 
151  
-    T object;
152  
-    bool global;
  245
+    // Don't bother if the scope isn't actually changing
  246
+    if (m_refType == type)
  247
+      return;
  248
+
  249
+    if(type == JNIGlobalRefType)
  250
+      reset((T)xbmc_jnienv()->NewGlobalRef(m_object));
  251
+
  252
+    else if (type == JNILocalRefType)
  253
+      reset((T)xbmc_jnienv()->NewLocalRef(m_object));
  254
+
  255
+    else if (type == JNIInvalidRefType)
  256
+      reset();
  257
+    m_refType = type;
  258
+  }
  259
+
  260
+  jobjectRefType m_refType;
  261
+  T m_object;
153 262
 };
154 263
 
155 264
 typedef jholder<jclass> jhclass;

0 notes on commit 44b79b4

Please sign in to comment.
Something went wrong with that request. Please try again.