Browse Source

Get rid of lock(this)

- Use readonly object as lock

Info from stackoverflow:

If I want to be sure that it will be locked for all threads inside my
application:
The lock object has to be static, if it locks access to static state.
Otherwise it has to be instance, because there's no need to lock state of
one class instance, and prevent other threads to work with another class
instance at the same time.

everyone says that the object has to be "readonly" I didn't found the
reason
Well, it doesn't have to be. This is just a best practice, which helps you
to avoid errors.

Signed-off-by: Syrone Wong <wong.syrone@gmail.com>
tags/3.3.2
Syrone Wong 8 years ago
parent
commit
46d95e7411
4 changed files with 18 additions and 7 deletions
  1. +4
    -1
      shadowsocks-csharp/Controller/Service/PortForwarder.cs
  2. +5
    -3
      shadowsocks-csharp/Controller/Service/TCPRelay.cs
  3. +4
    -1
      shadowsocks-csharp/Encryption/MbedTLSEncryptor.cs
  4. +5
    -2
      shadowsocks-csharp/View/LogForm.cs

+ 4
- 1
shadowsocks-csharp/Controller/Service/PortForwarder.cs View File

@@ -39,6 +39,9 @@ namespace Shadowsocks.Controller
// connection receive buffer
private byte[] connetionRecvBuffer = new byte[RecvSize];
// instance-based lock
private readonly object _Lock = new object();
public void Start(byte[] firstPacket, int length, Socket socket, int targetPort)
{
this._firstPacket = firstPacket;
@@ -220,7 +223,7 @@ namespace Shadowsocks.Controller
public void Close()
{
lock (this)
lock (_Lock)
{
if (_closed)
{


+ 5
- 3
shadowsocks-csharp/Controller/Service/TCPRelay.cs View File

@@ -160,8 +160,10 @@ namespace Shadowsocks.Controller
private bool _remoteShutdown = false;
private bool _closed = false;
private object _encryptionLock = new object();
private object _decryptionLock = new object();
// instance-based lock without static
private readonly object _encryptionLock = new object();
private readonly object _decryptionLock = new object();
private readonly object _closeConnLock = new object();
private DateTime _startConnectTime;
private DateTime _startReceivingTime;
@@ -207,7 +209,7 @@ namespace Shadowsocks.Controller
public void Close()
{
lock (this)
lock (_closeConnLock)
{
if (_closed) return;
_closed = true;


+ 4
- 1
shadowsocks-csharp/Encryption/MbedTLSEncryptor.cs View File

@@ -15,6 +15,9 @@ namespace Shadowsocks.Encryption
private IntPtr _encryptCtx = IntPtr.Zero;
private IntPtr _decryptCtx = IntPtr.Zero;
// instance based lock
private readonly object _Lock = new object();
public MbedTLSEncryptor(string method, string password, bool onetimeauth, bool isudp)
: base(method, password, onetimeauth, isudp)
{
@@ -120,7 +123,7 @@ namespace Shadowsocks.Encryption
protected virtual void Dispose(bool disposing)
{
lock (this)
lock (_Lock)
{
if (_disposed)
{


+ 5
- 2
shadowsocks-csharp/View/LogForm.cs View File

@@ -34,6 +34,9 @@ namespace Shadowsocks.View
const int BACK_OFFSET = 65536;
ShadowsocksController controller;
// global traffic update lock, make it static
private static readonly object _lock = new object();
#region chart
long lastMaxSpeed;
ShadowsocksController.QueueLast<TrafficInfo> traffic = new ShadowsocksController.QueueLast<TrafficInfo>();
@@ -71,7 +74,7 @@ namespace Shadowsocks.View
long maxSpeed = 0;
long lastInbound, lastOutbound;
lock (this)
lock (_lock)
{
if (traffic.Count == 0)
return;
@@ -120,7 +123,7 @@ namespace Shadowsocks.View
private void controller_TrafficChanged(object sender, EventArgs e)
{
lock (this)
lock (_lock)
{
traffic = new ShadowsocksController.QueueLast<TrafficInfo>();
foreach (var trafficPerSecond in controller.traffic)


Loading…
Cancel
Save