dup() golang FDs in obj-C code #213
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
At the moment, when using Code-Hex/vz like this:
```
f, err := os.OpenFile(devPath, open_flags, 0)
if err != nil {
return nil, fmt.Errorf("error opening file: %v", err)
}
attachment, err := vz.NewDiskBlockDeviceStorageDeviceAttachment(f, conf.ReadOnly, vz.DiskSynchronizationModeFull)
if err != nil {
_ = f.Close()
return nil, fmt.Errorf("error creating disk attachment: %v", err)
}
```
the developer has to know that `f` must be kept alive at least as long as the disk attachment is in use. If not, `f` will be garbage collected, its file descriptor will be closed, and the attachment becomes invalid. In this situation, the virtualization framework would return an error.
This commit simply adds a test case exhibiting this problem. This will
be improved in the next commits.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
…iptor In order to solve Code-Hex#201, this commit adds a `newFileHandleDupFd()` objective-C helper. This helper calls `dup()` on its file descriptor argument, and wraps it in a `NSFileHandle` with `closeOnDealloc:true`. This new file handle is fully independent from its golang counterpart, if the golang `os.File` holding the file descriptor is closed, the `NSFileHandle` will still be valid. This new helper is then used with vz.NewDiskBlockDeviceStorageDeviceAttachment. Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
…ment NewFileHandleNetworkDeviceAttachment has similar limitations as NewDiskBlockDeviceStorageDeviceAttachment. This commit fixes this by using newFileHandleDupFd to implement it. Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
NewFileHandleSerialPortAttachment has similar limitations as NewDiskBlockDeviceStorageDeviceAttachment. This commit fixes this by using newFileHandleDupFd to implement it. Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the issue described in #201.
The Code-Hex/vz APIs using an *os.File argument have a non-obvious limitation: the *os.File argument must be kept alive as long as the Code-Hex/vz object which uses them. If it was garbage collected, then this would cause errors.
This PR fixes this by ensuring the file descriptor is owned by Code-Hex/vz. This approach is backwards compatible with code that kept the
*os.Filealive.Which issue(s) this PR fixes:
Fixes #201
Additional documentation