The default bug view has changed. See this FAQ.

readString through CDataFinalizer

RESOLVED FIXED in mozilla15

Status

()

Core
js-ctypes
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Yoric, Assigned: Yoric)

Tracking

unspecified
mozilla15
x86
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

At the moment, readString may only be called on a CData object. We should be able to call it also on a CDataFinalizer.
Created attachment 618266 [details] [diff] [review]
Supporting readString in a CDataFinalizer
Assignee: nobody → dteller
Status: NEW → UNCONFIRMED
Ever confirmed: false
Attachment #618266 - Flags: review?(jorendorff)
Comment on attachment 618266 [details] [diff] [review]
Supporting readString in a CDataFinalizer

I don't see where test_finalizer_rel_string_t frees the memory allocated for the string in test_finalizer_acq_string_t. It's a leak, right?

>   acquire_void_ptr = library.declare("test_finalizer_acq_ptr_t",
>                                 ctypes.default_abi,
>-                                ctypes.int32_t.ptr,
>+                                ctypes.void_t.ptr,
>                                 ctypes.size_t);
>   dispose_void_ptr = library.declare("test_finalizer_rel_ptr_t",
>                                 ctypes.default_abi,
>                                 ctypes.void_t,
>                                 ctypes.int32_t.ptr);

While you're fixing acquire_void_ptr, I think the last line here (for dispose_void_ptr) should get the same change.

r=me with those changes.
Attachment #618266 - Flags: review?(jorendorff) → review+
Created attachment 620715 [details] [diff] [review]
Supporting readString in a CDataFinalizer

Fixed, thanks.
Attachment #618266 - Attachment is obsolete: true
Depends on: 748776
Created attachment 621510 [details] [diff] [review]
Supporting readString in a CDataFinalizer

Oops, turns out that this code could freeze – and that the testsuite somehow does not detect this. Now fixed.
Attachment #620715 - Attachment is obsolete: true
Created attachment 621511 [details] [diff] [review]
Supporting readString in a CDataFinalizer
Attachment #621510 - Attachment is obsolete: true
Created attachment 621515 [details] [diff] [review]
Supporting readString in a CDataFinalizer

Turns out that the error was caused by the interaction with the patch for bug 723530 – and that while it broke the xpcshell tests, it did so silently. Here is a patch that should solve the issue.
Attachment #621511 - Attachment is obsolete: true
Attachment #621515 - Flags: review?(jorendorff)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #621515 - Flags: review?(jorendorff) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/14ab85ab1549
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/14ab85ab1549
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.