[JAVA] Concurrency and hub connection state

[ncthbrt]

The latest version of the Java client on the /release/2.2 branch does not have measures in place to ensure that the hub connection state is synchronized across threads (with the naked variable being read without concurrency control), this leads to invoke calls failing even when the connection status is Connected on other threads.

A possible fix might be to mark this variable as volatile (though can at times be a fragile solution), or switch to using the accessor at all times, along with some form of locking.

[mikaelm12]

Hmm, interesting. We only update the actual HubConnectionState inside a lock in the client but I can do some investigation on this. Do you have any more information on a repro?
Thanks!

[ncthbrt]

Apologies, I'm not at a computer right now, but from the source it may be because you're using a rentrant lock rather than a reentrant read write lock, which as far as I understand means that reads much also be synchronised. I'm not a professional android developer however (work mainly in F#/C#), so understanding of Java threading semantics may be poor.

…

On Wed, 21 Nov 2018, 19:57 Mikael Mengistu ***@***.*** wrote: Hmm, interesting. We only update the actual HubConnectionState inside a lock in the client but I can do some investigation on this. Do you have any more information on a repro? Thanks! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3329 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AISSWVoj9hkF7lE91AklOXExGlleZ4m8ks5uxZQZgaJpZM4YtHaS> .

[ncthbrt]

Oh wait, read write locks also need lock acquisition for reads as well.

…

On Wed, 21 Nov 2018, 20:22 Nick Cuthbert ***@***.*** wrote: Apologies, I'm not at a computer right now, but from the source it may be because you're using a rentrant lock rather than a reentrant read write lock, which as far as I understand means that reads much also be synchronised. I'm not a professional android developer however (work mainly in F#/C#), soy understanding of Java threading semantics may be poor. On Wed, 21 Nov 2018, 19:57 Mikael Mengistu ***@***.*** wrote: > Hmm, interesting. We only update the actual HubConnectionState inside a > lock in the client but I can do some investigation on this. Do you have any > more information on a repro? > Thanks! > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#3329 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AISSWVoj9hkF7lE91AklOXExGlleZ4m8ks5uxZQZgaJpZM4YtHaS> > . >

[mikaelm12]

Hmm, well maybe would could transition to using a ReadWriteLock since it seems that we're not currently acquiring a lock on reads

[ncthbrt]

If need be, could take a look at this on Monday, and maybe make a PR?

[analogrelay]

#3330 was filed to track using a read/write lock

[mikaelm12]

@ncthbrt I'll take this on. We are considering back porting this change as well.
Thanks for your help!

[ncthbrt]

Of course.
Btw don't know if you saw the PR above.

[mikaelm12]

This issue is being addressed in this PR dotnet/aspnetcore#4400 and will be going into the 2.2.2 patch of the Java client. That issue is being tracked here:dotnet/aspnetcore#4393. Thank you for the contribution @ncthbrt!