raisedragon / pircbotx

Automatically exported from code.google.com/p/pircbotx
0 stars 0 forks source link

Bot becomes unable to stop after a while #154

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Start PircBotX with startBot
2. Wait
3. try to stop reconnecting with stopBotReconnect

What is the expected output? What do you see instead?

I expect reconnections to stop, but they don't.

What version of the product are you using? On what operating system?

2.0 through maven.

Please provide any additional information below.

because PircBotX.reconnectStopped is not volatile, the Hotspot optimizer 
eventually optimizes checking for it away and then it's impossible to ever stop 
the bot other than through System.exit. changing line 117 from

protected boolean reconnectStopped = false;

to

protected volatile boolean reconnectStopped = false;

will fix the issue.

Original issue reported on code.google.com by joerg.st...@gmail.com on 1 Dec 2013 at 8:14

GoogleCodeExporter commented 9 years ago
Why would it optimize the check away? There's lots of checks that would be 
optimized away if that's the case which would break hundreds of applications. 

And how would volatile fix this? Tbh I'm not too familiar with volatile but it 
seems it's more for threading issues, not optimization issues. 

Original comment by Lord.Qua...@gmail.com on 3 Dec 2013 at 3:04

GoogleCodeExporter commented 9 years ago
Hotspot optimizes it away because it thinks the variable is thread local, and 
it can see that it can't be changed from within the thread that is reading it 
(because startBot() never calls stopBotReconnect()). It is indeed a 
multithreading issue, which is why volatile would fix it.

Say thread A does bot.startBot, but then thread A blocks, because of the way 
startBot works. If you want to stop the bot, you have to call 
stopBotReconnect(), but since thread A is blocking, you have to call it from 
another thread (such as one of the event worker threads you spawn on incoming 
events, or the Swing EDT). But since the field reconnectStopped is not marked 
volatile, Java has no way to understand that it is being read from one thread 
(Thread A, which is doing the startBot loop) while being written from another 
thread.

Basically, this is the reason for the volatile keyword to exist - a variable 
that is written to from one thread and read from another. The problem could 
also be fixed by changing stopBotReconnect() to somehow synchronize on thread 
A, but I can think of no easy way of doing that, as startBot, or rather the 
connect() it calls is blocking.

Hotspot only optimizes cases like this after an unspecified amounts of loops 
(in this case, reconnects), depending on JVM, CPU load, loop count and OS, so 
it's not an issue that is easy to reproduce short of waiting a few days while 
being connected to an unstable server.

Original comment by joerg.st...@gmail.com on 3 Dec 2013 at 3:14

GoogleCodeExporter commented 9 years ago
That's probably why I never ran into the issue during testing. Fixed in 
Revision 5951ca93d6bf .

Original comment by Lord.Qua...@gmail.com on 3 Dec 2013 at 3:28

GoogleCodeExporter commented 9 years ago
Thanks for the fast response.

Original comment by joerg.st...@gmail.com on 3 Dec 2013 at 3:30