search

rubenlagus / TelegramBots

Java library to create bots using Telegram Bots API
https://telegram.me/JavaBotsApi
MIT License
4.68k stars 1.18k forks source link

Bot closing is not handled well #142

Closed laptevn closed 7 years ago

laptevn commented 8 years ago

I didn't find an example of correct closing the bot. So I used the following code:

public class Main {
    public static void main(String[] args) {
        TelegramBotsApi telegramBotsApi = new TelegramBotsApi();
        BotSession session = null;
        try {
            session = telegramBotsApi.registerBot(new Bot());
            new Scanner(System.in).nextLine();
        } catch (TelegramApiException e) {
            e.printStackTrace();
        } finally {
            if (session != null) {
                session.close();
            }
        }
    }
}

That gives me InterruptedException and SocketException logged in stdout. Sep 03, 2016 12:35:55 PM org.telegram.telegrambots.logging.BotLogger severe SEVERE: BOTSESSION java.lang.InterruptedException at java.lang.Object.wait(Native Method) at java.lang.Object.wait(Object.java:502) at org.telegram.telegrambots.updatesreceivers.BotSession$HandlerThread.run(BotSession.java:186)

Sep 03, 2016 12:35:55 PM org.telegram.telegrambots.logging.BotLogger severe SEVERE: BOTSESSION java.net.SocketException: Socket closed at java.net.SocketInputStream.socketRead0(Native Method) at java.net.SocketInputStream.socketRead(SocketInputStream.java:116) at java.net.SocketInputStream.read(SocketInputStream.java:170) at java.net.SocketInputStream.read(SocketInputStream.java:141) at sun.security.ssl.InputRecord.readFully(InputRecord.java:465) at sun.security.ssl.InputRecord.read(InputRecord.java:503) at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:961) at sun.security.ssl.SSLSocketImpl.readDataRecord(SSLSocketImpl.java:918) at sun.security.ssl.AppInputStream.read(AppInputStream.java:105) at org.apache.http.impl.io.SessionInputBufferImpl.streamRead(SessionInputBufferImpl.java:139) at org.apache.http.impl.io.SessionInputBufferImpl.fillBuffer(SessionInputBufferImpl.java:155) at org.apache.http.impl.io.SessionInputBufferImpl.readLine(SessionInputBufferImpl.java:284) at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:140) at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:57) at org.apache.http.impl.io.AbstractMessageParser.parse(AbstractMessageParser.java:261) at org.apache.http.impl.DefaultBHttpClientConnection.receiveResponseHeader(DefaultBHttpClientConnection.java:165) at org.apache.http.impl.conn.CPoolProxy.receiveResponseHeader(CPoolProxy.java:167) at org.apache.http.protocol.HttpRequestExecutor.doReceiveResponse(HttpRequestExecutor.java:272) at org.apache.http.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:124) at org.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:271) at org.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:184) at org.apache.http.impl.execchain.RetryExec.execute(RetryExec.java:88) at org.apache.http.impl.execchain.RedirectExec.execute(RedirectExec.java:110) at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:184) at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82) at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:107) at org.telegram.telegrambots.updatesreceivers.BotSession$ReaderThread.run(BotSession.java:128)

Sep 03, 2016 12:35:55 PM org.telegram.telegrambots.logging.BotLogger severe SEVERE: BOTSESSION java.lang.InterruptedException at java.lang.Object.wait(Native Method) at org.telegram.telegrambots.updatesreceivers.BotSession$ReaderThread.run(BotSession.java:166)

After a quick look into source code I found that Thread interrupted status is not handled well. This API should have a way to close resources well so that no memory/resource leaks exist.

rubenlagus commented 7 years ago

Hi @laptevn, can you have a look to last version? In any case, those exceptions you see are nothing to worry. Just the InterruptedException fired when I interrupt Handler/Reader thread (as part of the closing process) and the SocketException when the connection is closed (also correct).

laptevn commented 7 years ago

Hi @rubenlagus , Checked version 2.4.0 from Maven central. Still the same issue. I would recommend to return to fundamentals and reconsider general approach to exception handling, multithreading handling and resource handling. Exceptions always say that there is something wrong. And there is always a correct way to handle resources without generating exceptions. Both streams from Java API and Apache HTTP client let us do that.

rubenlagus commented 7 years ago

Well, how do you suggest to close an HttpClient if it is not calling close()method? I'm open to suggestion.

Also I don't think there is any issue in logging the InterruptedException as long as there is nothing left (that I can't see in the code). If you feel better, I can just change the log so it only writes "Hi, the thread was interrupted" instead of printing the stacktrace, but I don't see any problem on having the full stacktrace in a debug logging level.

rubenlagus commented 7 years ago

@laptevn Has this been improved in latest versions? Do you have any suggestion or can we close it?