Because of #808 I decided to check some other fields too and noticed that a similar issue affects connectionLostTimeout field (AbstractWebSocket:68) and it is not documented. I'm sure there are other places like this with potential for bugs (setAttachment/getAttachment?).
Example code which demonstrates this:
MyServer server = new MyServer(new InetSocketAddress(12345));
server.start();
new Thread(() -> {
while(server.getConnectionLostTimeout() == 60) {
// do nothing
}
// will never reach this statement
System.out.println("Finished!");
}).start();
Thread.sleep(1000);
server.setConnectionLostTimeout(20);While this particulary example isn't a common scenario, there some things I'm worried about:
- This library internally uses lots of different threads and it is difficult to ensure every read and write occurs in a synchronized context.
- There isn't any documentation on thread-safety of classes/methods provided by this library. Many users could assume that thread safety is already provided and introduce bugs in their programs.
- Potential issues will be difficult to reproduce because the result depends on exact order of reads. Here is a worst-case scenario: replace
Thread.sleep(1000) in the example with
var bytes = new byte[N];
new Random().nextBytes(bytes);
Arrays.sort(bytes);
On my computer, if N is 1000, the action happens quickly enough so that main thread writes before other thread reads and it prints "Finished", but if N is 1_000_000, it doesn't and the other thread keeps spinning forever.
Summary
Thread safety of various methods and classes should be checked, optionally fixed and clearly documented. The most important places to start would be code exposed to users, but the internal code should be documented as well.
Because of #808 I decided to check some other fields too and noticed that a similar issue affects connectionLostTimeout field (AbstractWebSocket:68) and it is not documented. I'm sure there are other places like this with potential for bugs (setAttachment/getAttachment?).
Example code which demonstrates this:
While this particulary example isn't a common scenario, there some things I'm worried about:
Thread.sleep(1000)in the example withOn my computer, if N is 1000, the action happens quickly enough so that main thread writes before other thread reads and it prints "Finished", but if N is 1_000_000, it doesn't and the other thread keeps spinning forever.
Summary
Thread safety of various methods and classes should be checked, optionally fixed and clearly documented. The most important places to start would be code exposed to users, but the internal code should be documented as well.