-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Remove warnings #102
base: master
Are you sure you want to change the base?
WIP: Remove warnings #102
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rebase this on the merged 5.2 for starters?
e16dd51
to
c8d4a96
Compare
@@ -21,20 +21,22 @@ public final class Callbacker<Value> { | |||
|
|||
private var callbacks = Callbacks.none | |||
private var _mutex = pthread_mutex_t() | |||
private var mutex: PThreadMutex { return PThreadMutex(&_mutex) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you allowed to add extension to PThreadMutex (UnsafeMutablePointer<pthread_mutex_t>)? Otherwise perhaps create a thin wrapper around it and use that instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea! I'll try that!
0397903
to
d26589b
Compare
} | ||
|
||
mutating func unlock() { | ||
withPointer { $0.unlock() } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not add protect
as well, to avoid rewriting a lot of code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It caused crashes when I used protect
. I'm not a 100% sure why though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did your code look like and did the log specify why it crashed (e.g. exclusivity violation)?
@@ -10,8 +10,7 @@ import Foundation | |||
|
|||
/// A reference wrapper around a POSIX thread mutex | |||
public final class Mutex { | |||
private var _mutex = pthread_mutex_t() | |||
private var mutex: PThreadMutex { return PThreadMutex(&_mutex) } | |||
private var mutex = pthread_mutex_t() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw this comment:
/// Helper methods to work directly with a Pthread mutex pointer to avoid overhead of alloction and reference counting of using the Mutex reference type.
/// - Note: You have to explicity call `initialize()` before use (typically in a class init) and `deinitialize()` when done (typically in a class deinit)
extension UnsafeMutablePointer where Pointee == pthread_mutex_t {
I'm trying to remember what the reason we the pthread_mutex_t
-> PThreadMutex
dance above, and if this change will alter the behaviour are performance...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, ok this solution might not be correct at all. I thought that because private var mutex: PThreadMutex { return PThreadMutex(&_mutex) }
would create a new UnsafeMutablePointer
every time you called it. It would be functionally the same as withUnsafeMutablePointer(to: &_mutex) { ... }
but it seems to change behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@niil-ohlin do you remember why this didn't work? I opened #104 now and saw your PR
5eaa296
to
797072b
Compare
deccedd
to
65d4f58
Compare
A solution for removing warnings. I really don't like this solutions. It's way to naive and hacky and really degrades the quality of the codebase. Does anyone have a suggestion for how the mutex warnings can be addressed?