shayhatsor / zookeeper

Apache ZooKeeper .NET async Client
https://nuget.org/packages/ZooKeeperNetEx/
Apache License 2.0
236 stars 53 forks source link

Make Fenced and SignalTask public #30

Closed bitchkat closed 6 years ago

bitchkat commented 6 years ago

Please consider making Fenced and SignalTask (especially this) public instead of internal. Curator InterProcessMutex uses java wait/notifyAll which normally would be converted to Monitor.Wait/Monitor.PulseAll except it needs to call PulseAll in a watcher that doesn't lock.

SignalTask looks like it provides the appropriate type of signaling needed and I would prefer to not make a copy it.

shayhatsor commented 6 years ago

no problem, will do

bitchkat commented 6 years ago

Thanks. I've added some convenience methods that I find handy for using SignalTask

` ///

/// Set the signal and reset in an atomic operation. /// public void SetAndReset() { var completion = tcs.Value; Reset(); completion.TrySetResult(true); }

    /// <summary>
    /// Wait for the signal to be set
    /// </summary>
    public void Wait()
    {
        tcs.Value.Task.Wait();
    }

    /// <summary>
    /// Wait for the signal to set with a timeout
    /// </summary>
    /// <param name="timeout">amount of time in milliseconds to wait for the signal</param>
    /// <returns>true if signal was received</returns>
    public bool Wait(int timeout)
    {
        return tcs.Value.Task.Wait(timeout);
    }`
shayhatsor commented 6 years ago

@bitchkat, I've made SignalTask public. The methods you've suggested might be handy, but my usage didn't warrant adding them (I suggest extension methods). About Fenced, it is an over the top over-optimization that probably works the same as putting a lock - so i'd rather keep it internal.