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
Update dependencies, remove deprecations #157
Conversation
Codecov Report
@@ Coverage Diff @@
## master #157 +/- ##
============================================
+ Coverage 18.54% 20.26% +1.72%
- Complexity 80 82 +2
============================================
Files 32 30 -2
Lines 1111 1120 +9
Branches 116 106 -10
============================================
+ Hits 206 227 +21
+ Misses 885 873 -12
Partials 20 20
Continue to review full report at Codecov.
|
@@ -673,7 +669,7 @@ void stop() | |||
/** | |||
* Stores our last MUC presence packet for future update. | |||
*/ | |||
private Presence lastPresenceSent; | |||
private PresenceBuilder lastPresenceSent; |
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.
In Smack, Stanza modification is synchronized, but builders are not. I haven't analyzed the threading model here, but is there any danger with this?
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.
Hm, good question. What do we actually need/want to keep from the presence packet, i.e. why is it intercepted and stored instead of always built from scratch?
I could synchronize on the builder while modifying the extension list and while calling .build(), but that's ugly :-/
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'm really not sure who calls what here. There's some synchronization from MucClient and MucClientManager, but that's unlikely to be meant and usable for the presence object. I've added some synchronization that should be cheap enough to and not interfere with Smack.
¹ actually, still somewhat used in Jigasi, but other OSGi things that were still used were already removed in e.g. #132. jitsi/jigasi#387 will handle this.