goxr3plus / goxr3plus/java-stream-player

Questionable redefinition of audioInputStream

Open
#55 0 comments 0 reactions 2 assignees Claimed by @goxr3plus View on GitHub
question
Dominant language
Java
Stars
158
Forks
36
PR merge metrics
No merged PRs in 30d

Description

This looks a bit weird in my eyes

https://github.com/goxr3plus/java-stream-player/blob/6a00a79f32bf324f7d6232182d93745da64bee0b/src/main/java/com/goxr3plus/streamplayer/stream/StreamPlayer.java#L506-L507

audioInputStream is a field which is given a value in initAudioInputStream()
https://github.com/goxr3plus/java-stream-player/blob/6a00a79f32bf324f7d6232182d93745da64bee0b/src/main/java/com/goxr3plus/streamplayer/stream/StreamPlayer.java#L330-L331

Then it is redefined, i.e., it loses its old value and get a new one. Why isn't a different variable (field) used for the new value?

There is another issue with the assignment on line 507:

Non-atomic operation on volatile field 'audioInputStream'
Inspection info: Reports any non-atomic operations on volatile fields. Non-atomic operations on volatile fields are operations where the field is read and the value is used to update the field. It is possible for the value of the field to change between the read and the write, possibly invalidating the operation. The non-atomic operation can be avoided by surrounding it with a synchronized block or by making use of one of the classes from the java.util.concurrent.atomic package.

As mentioned earlier, it's probably a mistake to make audioInputStream volatile. SonarQube issues a warning where the volatile field is declared. For a description, see https://wiki.sei.cmu.edu/confluence/display/java/CON50-J.+Do+not+assume+that+declaring+a+reference+volatile+guarantees+safe+publication+of+the+members+of+the+referenced+object

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.