.NET (Core and Framework)

Collin Jasnoch wrote:

Why does it need an object just for locking?

private object m_objVideoPlayerSync = new object();
private List<string> m_lstVideos = new List<string>();
private EnumPlaybackState m_playbackState = EnumPlaybackState.Stopped;

private void AddVideoToList(string szVideoFileName)
{
    lock(m_objVideoPlayerSync)
    {
        m_lstVideos.Add(szVideoFileName);
    }
}

private void PlayNextVideo()
{
    lock(m_objVideoPlayerSync)
    {
        string szVideo = m_lstVideos[0];
        m_lstVideos.RemoveAt(0);
        m_playbackState = EnumPlaybackState.Playback;
        StartPlayBack();
    }
}

private void StopVideo()
{
    lock(m_objVideoPlayerSync)
    {
        StopPlayBack();
        m_playbackState = EnumPlaybackState.Stopped;
    }
}

string szVideo = string.Empty;
lock(m_lstVideos)
{
    szVideo = m_lstVideos[0];
    m_lstVideos.RemoveAt(0);
}

lock(m_objVideoPlayerSync)
{
    m_playbackState = EnumPlaybackState.Playback;
    StartPlayBack();
}

In general, avoid locking on a public type, or instances beyond your code's control. The common constructs lock (this), lock (typeof (MyType)), and lock ("myLock") violate this guideline:

lock (this) is a problem if the instance can be accessed publicly.

lock (typeof (MyType)) is a problem if MyType is publicly accessible.

lock(“myLock”) is a problem since any other code in the process using the same string, will share the same lock.

Best practice is to define a private object to lock on, or a private static object variable to protect data common to all instances.

Luc Pattyn wrote:

All is fine if the outside world cannot touch the lock object, so if you have say a return _collection; somewhere, then the first is no good.