Skip to content

Commit ffdffab

Browse files
committed
WebIDL: Implement Owner Extended Attribute, fix #77
Currently the ensureCache create a temporary pointer that will transfer the value to the WASM/C part and recycle it with to use in the next value, but since the libass struct pointers are owned by the library, the pointers can't be recycled or freed or can lead into an undefined behavior. This configure the binder to tranfer the pointer ownership instead of recycle it. To avoid creating complex code, I decided to fix it in the webidl binder.
1 parent d1072f3 commit ffdffab

File tree

3 files changed

+62
-36
lines changed

3 files changed

+62
-36
lines changed

build/WebIDL.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2863,6 +2863,7 @@ def handleExtendedAttribute(self, attr):
28632863
identifier == "AvailableIn" or
28642864
identifier == "Const" or
28652865
identifier == "Value" or
2866+
identifier == "Owner" or
28662867
identifier == "BoundsChecked" or
28672868
identifier == "NewObject"):
28682869
# Known attributes that we don't need to do anything with here
@@ -2938,7 +2939,7 @@ def addExtendedAttributes(self, attrs):
29382939
self.enforceRange = True
29392940
elif identifier == "TreatNonCallableAsNull":
29402941
self._allowTreatNonCallableAsNull = True
2941-
elif identifier in ['Ref', 'Const']:
2942+
elif identifier in ['Ref', 'Const', 'Owner']:
29422943
# ok in emscripten
29432944
self._extraAttributes[identifier] = True
29442945
else:

build/webidl_binder.py

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ def build_constructor(name):
174174
size: 0, // the size of buffer
175175
pos: 0, // the next free offset in buffer
176176
temps: [], // extra allocations
177+
owned: [], // Owned allocations
177178
needed: 0, // the total size we need next time
178179
179180
prepare: function() {
@@ -197,22 +198,29 @@ def build_constructor(name):
197198
}
198199
ensureCache.pos = 0;
199200
},
200-
alloc: function(array, view) {
201+
alloc: function(array, view, owner) {
201202
assert(ensureCache.buffer);
202203
var bytes = view.BYTES_PER_ELEMENT;
203204
var len = array.length * bytes;
204205
len = (len + 7) & -8; // keep things aligned to 8 byte boundaries
205206
var ret;
206-
if (ensureCache.pos + len >= ensureCache.size) {
207-
// we failed to allocate in the buffer, ensureCache time around :(
207+
if (owner) {
208208
assert(len > 0); // null terminator, at least
209209
ensureCache.needed += len;
210210
ret = Module['_malloc'](len);
211-
ensureCache.temps.push(ret);
211+
ensureCache.owned.push(ret);
212212
} else {
213-
// we can allocate in the buffer
214-
ret = ensureCache.buffer + ensureCache.pos;
215-
ensureCache.pos += len;
213+
if (ensureCache.pos + len >= ensureCache.size) {
214+
// we failed to allocate in the buffer, ensureCache time around :(
215+
assert(len > 0); // null terminator, at least
216+
ensureCache.needed += len;
217+
ret = Module['_malloc'](len);
218+
ensureCache.temps.push(ret);
219+
} else {
220+
// we can allocate in the buffer
221+
ret = ensureCache.buffer + ensureCache.pos;
222+
ensureCache.pos += len;
223+
}
216224
}
217225
return ret;
218226
},
@@ -228,58 +236,73 @@ def build_constructor(name):
228236
view[offset + i] = array[i];
229237
}
230238
},
239+
clear: function(clearOwned) {
240+
for (var i = 0; i < ensureCache.temps.length; i++) {
241+
Module['_free'](ensureCache.temps[i]);
242+
}
243+
if (clearOwned) {
244+
for (var i = 0; i < ensureCache.owned.length; i++) {
245+
Module['_free'](ensureCache.owned[i]);
246+
}
247+
}
248+
ensureCache.temps.length = 0;
249+
Module['_free'](ensureCache.buffer);
250+
ensureCache.buffer = 0;
251+
ensureCache.size = 0;
252+
ensureCache.needed = 0;
253+
}
231254
};
232255
233256
/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant) */
234-
function ensureString(value) {
257+
function ensureString(value, owner) {
235258
if (typeof value === 'string') {
236259
var intArray = intArrayFromString(value);
237-
var offset = ensureCache.alloc(intArray, HEAP8);
260+
var offset = ensureCache.alloc(intArray, HEAP8, owner);
238261
ensureCache.copy(intArray, HEAP8, offset);
239262
return offset;
240263
}
241264
return value;
242265
}
243266
/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant) */
244-
function ensureInt8(value) {
267+
function ensureInt8(value, owner) {
245268
if (typeof value === 'object') {
246-
var offset = ensureCache.alloc(value, HEAP8);
269+
var offset = ensureCache.alloc(value, HEAP8, owner);
247270
ensureCache.copy(value, HEAP8, offset);
248271
return offset;
249272
}
250273
return value;
251274
}
252275
/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant) */
253-
function ensureInt16(value) {
276+
function ensureInt16(value, owner) {
254277
if (typeof value === 'object') {
255-
var offset = ensureCache.alloc(value, HEAP16);
278+
var offset = ensureCache.alloc(value, HEAP16, owner);
256279
ensureCache.copy(value, HEAP16, offset);
257280
return offset;
258281
}
259282
return value;
260283
}
261284
/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant) */
262-
function ensureInt32(value) {
285+
function ensureInt32(value, owner) {
263286
if (typeof value === 'object') {
264-
var offset = ensureCache.alloc(value, HEAP32);
287+
var offset = ensureCache.alloc(value, HEAP32, owner);
265288
ensureCache.copy(value, HEAP32, offset);
266289
return offset;
267290
}
268291
return value;
269292
}
270293
/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant) */
271-
function ensureFloat32(value) {
294+
function ensureFloat32(value, owner) {
272295
if (typeof value === 'object') {
273-
var offset = ensureCache.alloc(value, HEAPF32);
296+
var offset = ensureCache.alloc(value, HEAPF32, owner);
274297
ensureCache.copy(value, HEAPF32, offset);
275298
return offset;
276299
}
277300
return value;
278301
}
279302
/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant) */
280-
function ensureFloat64(value) {
303+
function ensureFloat64(value, owner) {
281304
if (typeof value === 'object') {
282-
var offset = ensureCache.alloc(value, HEAPF64);
305+
var offset = ensureCache.alloc(value, HEAPF64, owner);
283306
ensureCache.copy(value, HEAPF64, offset);
284307
return offset;
285308
}
@@ -390,7 +413,7 @@ def type_to_cdec(raw):
390413

391414
def render_function(class_name, func_name, sigs, return_type, non_pointer,
392415
copy, operator, constructor, func_scope,
393-
call_content=None, const=False, array_attribute=False):
416+
call_content=None, const=False, owned=False, array_attribute=False):
394417
legacy_mode = CHECKS not in ['ALL', 'FAST']
395418
all_checks = CHECKS == 'ALL'
396419

@@ -505,20 +528,20 @@ def render_function(class_name, func_name, sigs, return_type, non_pointer,
505528
if not (arg.type.isArray() and not array_attribute):
506529
body += " if ({0} && typeof {0} === 'object') {0} = {0}.ptr;\n".format(js_arg)
507530
if arg.type.isString():
508-
body += " else {0} = ensureString({0});\n".format(js_arg)
531+
body += " else {0} = ensureString({0}, {1});\n".format(js_arg, "true" if (owned) else "false")
509532
else:
510533
# an array can be received here
511534
arg_type = arg.type.name
512535
if arg_type in ['Byte', 'Octet']:
513-
body += " if (typeof {0} == 'object') {{ {0} = ensureInt8({0}); }}\n".format(js_arg)
536+
body += " if (typeof {0} == 'object') {{ {0} = ensureInt8({0}, {1}); }}\n".format(js_arg, "true" if (owned) else "false")
514537
elif arg_type in ['Short', 'UnsignedShort']:
515-
body += " if (typeof {0} == 'object') {{ {0} = ensureInt16({0}); }}\n".format(js_arg)
538+
body += " if (typeof {0} == 'object') {{ {0} = ensureInt16({0}, {1}); }}\n".format(js_arg, "true" if (owned) else "false")
516539
elif arg_type in ['Long', 'UnsignedLong']:
517-
body += " if (typeof {0} == 'object') {{ {0} = ensureInt32({0}); }}\n".format(js_arg)
540+
body += " if (typeof {0} == 'object') {{ {0} = ensureInt32({0}, {1}); }}\n".format(js_arg, "true" if (owned) else "false")
518541
elif arg_type == 'Float':
519-
body += " if (typeof {0} == 'object') {{ {0} = ensureFloat32({0}); }}\n".format(js_arg)
542+
body += " if (typeof {0} == 'object') {{ {0} = ensureFloat32({0}, {1}); }}\n".format(js_arg, "true" if (owned) else "false")
520543
elif arg_type == 'Double':
521-
body += " if (typeof {0} == 'object') {{ {0} = ensureFloat64({0}); }}\n".format(js_arg)
544+
body += " if (typeof {0} == 'object') {{ {0} = ensureFloat64({0}, {1}); }}\n".format(js_arg, "true" if (owned) else "false")
522545

523546
c_names = {}
524547
for i in range(min_args, max_args):
@@ -737,6 +760,7 @@ def render_function(class_name, func_name, sigs, return_type, non_pointer,
737760
func_scope=interface,
738761
call_content=get_call_content,
739762
const=m.getExtendedAttribute('Const'),
763+
owned=m.getExtendedAttribute('Owner'),
740764
array_attribute=m.type.isArray())
741765

742766
if m.readonly:
@@ -755,6 +779,7 @@ def render_function(class_name, func_name, sigs, return_type, non_pointer,
755779
func_scope=interface,
756780
call_content=set_call_content,
757781
const=m.getExtendedAttribute('Const'),
782+
owned=m.getExtendedAttribute('Owner'),
758783
array_attribute=m.type.isArray())
759784
mid_js += [r'''
760785
Object.defineProperty(%s.prototype, '%s', { get: %s.prototype.%s, set: %s.prototype.%s });''' % (name, attr, name, get_name, name, set_name)]

src/SubtitleOctopus.idl

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ interface ASS_Image {
1212

1313
[NoDelete]
1414
interface ASS_Style {
15-
attribute DOMString Name;
16-
attribute DOMString FontName;
15+
[Owner] attribute DOMString Name;
16+
[Owner] attribute DOMString FontName;
1717
attribute double FontSize;
1818
attribute unsigned long PrimaryColour;
1919
attribute unsigned long SecondaryColour;
@@ -47,12 +47,12 @@ interface ASS_Event {
4747
attribute long ReadOrder;
4848
attribute long Layer;
4949
attribute long Style;
50-
attribute DOMString Name;
50+
[Owner] attribute DOMString Name;
5151
attribute long MarginL;
5252
attribute long MarginR;
5353
attribute long MarginV;
54-
attribute DOMString Effect;
55-
attribute DOMString Text;
54+
[Owner] attribute DOMString Effect;
55+
[Owner] attribute DOMString Text;
5656
};
5757

5858
[NoDelete]
@@ -63,17 +63,17 @@ interface ASS_Track {
6363
attribute long max_events;
6464
[Value] attribute ASS_Style[] styles;
6565
[Value] attribute ASS_Event[] events;
66-
attribute DOMString style_format;
67-
attribute DOMString event_format;
66+
[Owner] attribute DOMString style_format;
67+
[Owner] attribute DOMString event_format;
6868
attribute long PlayResX;
6969
attribute long PlayResY;
7070
attribute double Timer;
7171
attribute long WrapStyle;
7272
attribute long ScaledBorderAndShadow;
7373
attribute long Kerning;
74-
attribute DOMString Language;
74+
[Owner] attribute DOMString Language;
7575
attribute long default_style;
76-
attribute DOMString name;
76+
[Owner] attribute DOMString name;
7777
};
7878

7979
enum ASS_Hinting {

0 commit comments

Comments
 (0)