Move ipc::FileDescriptor's UniquePtr instance into MFBT so it can be used more widely
Categories
(Core :: IPC, enhancement, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: jld, Assigned: jld)
References
Details
Attachments
(2 files)
Part of my patch stack for bug 1479960 is to clean up some of how we manage file descriptors / handles to avoid leaks and confusion about ownership. (Freezing is modeled as changing the permissions on the underlying object, because that's all we can do in some cases, but in other cases that has to be simulated by creating a new handle and closing the previous one while ensuring no copies have accidentally escaped.) Specifically, we have a UniquePtr instance for this in ipc::FileDescriptor, but there are use cases that can't or shouldn't be depending on those headers, and I'm told that this would be in scope for mfbt/UniquePtrExtensions.h.
So I did that and used it in bug 1479960, but it feels distinct enough that I thought a separate bug would make more sense.
|
Assignee | |
Comment 1•5 years ago
|
||
MozReview-Commit-ID: 7bbGVIjTTaJ
|
|
Assignee | |
Comment 2•5 years ago
|
||
Now that UniqueFileHandle can be used more widely, and with
ipc::FileDescriptor being essentially a copyable UniqueFileHandle, it
makes sense to add a move constructor and a "forget"-like method to
convert between them when needed.
Depends on D26737
|
||
Comment 3•5 years ago
|
||
There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jld, could you have a look please?
For more information, please visit auto_nag documentation.
|
|
Assignee | |
Comment 4•5 years ago
|
||
(In reply to Release mgmt bot [:sylvestre / :calixte] from comment #3)
There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jld, could you have a look please?
For more information, please visit auto_nag documentation.
Once again, it's part of a larger patch stack that's not done yet.
Pushed by jedavis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/771db331860a Move ipc::FileDescriptor's UniquePtr instance into MFBT as UniqueFileHandle. r=froydnj https://hg.mozilla.org/integration/autoland/rev/7fba719355d7 Add free interconversion between UniqueFileHandle and ipc::FileDescriptor and use it in a few places. r=froydnj
Pushed by jedavis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/26bb00a94d5d Move ipc::FileDescriptor's UniquePtr instance into MFBT as UniqueFileHandle. r=froydnj https://hg.mozilla.org/integration/autoland/rev/3c2b31744645 Add free interconversion between UniqueFileHandle and ipc::FileDescriptor and use it in a few places. r=froydnj
|
||
Comment 7•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/26bb00a94d5d
https://hg.mozilla.org/mozilla-central/rev/3c2b31744645
|
||
Comment 8•5 years ago
|
||
Backed out for toolchain bustages on UniquePtrExtensions.h
Backout link: https://hg.mozilla.org/integration/autoland/rev/56834c5fc9390da3bdfb41a3fe3f06119fbb3502
Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=a8518ea4b5944cd5563a882c27b304fc82d07dba&selectedJob=253975140
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253975140&repo=autoland&lineNumber=922
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 9•5 years ago
|
||
The build breakage fix is bug 1562358. Try runs before and after, based on a rev before the backout was merged to m-c. I'll reland after the merge next Monday.
|
||
Comment 10•5 years ago
|
||
Pushed by jedavis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a628ca96446d Move ipc::FileDescriptor's UniquePtr instance into MFBT as UniqueFileHandle. r=froydnj https://hg.mozilla.org/integration/autoland/rev/c9d25298b6f6 Add free interconversion between UniqueFileHandle and ipc::FileDescriptor and use it in a few places. r=froydnj
|
||
Comment 11•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a628ca96446d
https://hg.mozilla.org/mozilla-central/rev/c9d25298b6f6
Description
•