Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[QUESTION] Why do process handles keep increasing. #185

Open
yjs112233 opened this issue Aug 23, 2021 · 21 comments
Open

[QUESTION] Why do process handles keep increasing. #185

yjs112233 opened this issue Aug 23, 2021 · 21 comments
Labels
bug

Comments

@yjs112233
Copy link

[describe]
--version , demo download from this.
--start from bt.cli.CliClient.
--After running multiple instances[new CliClient()] in the same process and all in shutdown, the handle still exists. Maybe somewhere forgot to close stream.
--other. A large number of connections were found in CLOSE_WAIT state.

If this is a problem, how can I solve it to keep the program running consistently?
Thanks.
@yjs112233 yjs112233 added the bug label Aug 23, 2021
@pyckle
Copy link
Collaborator

pyckle commented Oct 4, 2021

Hey @yjs112233

CliClient is designed to be used from the command line, not programatically. The runtime shutdown is disabled:

bt/bt-cli/src/main/java/bt/cli/CliClient.java

Line 139 in 7a141ed

.disableAutomaticShutdown()

Is there a reason why you're using the CliClient class rather than directly integrating with BtClient/BtRuntime?

@yjs112233
Copy link
Author

@pyckle

Thank you very much for paying attention to this problem that has bothered me for several months.
In fact, I've been using BitTorrent for almost a year. At first, I integrated it into my application to receive BT file parsing and download. However, after integration, I found that the same BT file cannot be received in the same BtRuntime, and it will stop running because it has been registered. Therefore, I changed the integration mode. The parsing of a BT file will start a BtRuntime and mount a unique BtClient (even though BtRuntime can attch many BtClients in design). After the BT download is completed, let it close itself.
According to this idea, on the BtRuntime configuration, do not use .disableAutomaticShutdown() so that it will shutdown automatically when it is completed. Each BtRuntime uses the same pair of port numbers 6891 and 49001.
Here is my BtRuntime configuration:

public BtRuntime getBtRuntime(){
BtRuntimeBuilder runtimeBuilder = getBtRuntimeBuilder(6891,49001);
BtRuntime runtime = runtimeBuilder.build();
runtime.startup();
return runtime;
}

private BtRuntimeBuilder getBtRuntimeBuilder(int port, int dhtPort){
    return BtRuntime.builder(this.getConfig(port))
                                 .module(this.getDHTMpdule(dhtPort))
                                 .autoLoadModules();
}
private Config getConfig(int port){
Config config = new Config() {
@OverRide
public InetAddress getAcceptorAddress() {
return super.getAcceptorAddress();
}

        @Override
        public int getAcceptorPort() {
            return port;
        }

        @Override
        public int getNumOfHashingThreads() {
            return Runtime.getRuntime().availableProcessors() * 2;
        }

        @Override
        public EncryptionPolicy getEncryptionPolicy() {
            return EncryptionPolicy.REQUIRE_ENCRYPTED;
        }
    };
    return config;
}
private Module getDHTMpdule(int dhtPort){
Module dhtModule = new DHTModule(new DHTConfig() {
@OverRide
public int getListeningPort() {
return dhtPort;
}

        @Override
        public boolean shouldUseRouterBootstrap() {
            return true;
        }
    });
    return dhtModule;
}

Here is my BtClient configuration:

public BtClient torrentClient(Torrent torrent,String id, List fileList, BtRuntime btRuntime){
if (fileList == null || fileList.isEmpty()){
throw new TorrentException(TorrentResultEnum.SELECTOR_IS_NULL);
}
String magnet = TorrentUtils.toMagnetLink(torrent);
magnet = addLatestTrackers(magnet);
magnet = TorrentUtils.magnetTrim(magnet);
BtFileSelector selector = new BtFileSelector(fileList);
BtClientBuilder builder = Bt.client(btRuntime)
.fileSelector(selector)
.magnet(magnet)
.storage(this.getStorage(id))
.randomizedRarestSelector();
return builder.build();
}

At first everything was normal. And it can support multiple tasks at the same time.
However, with the gradual increase of tasks, more and more too many open files exceptions appear in my application.
For further confirmation, I only used bt.cli.cliclient. To reproduce this problem, and found that it was the same.
Back to the above question, is it my configuration error or a bug in BitTorrent.

@yjs112233
Copy link
Author

I use BT version 1.9 from maven repository, the self-contained org. Eclipse. Jetty is excluded. And additional integration with org.eclipse.jetty version： 8.2.0.v20160908；

@atomashpolskiy
Copy link
Owner

atomashpolskiy commented Oct 5, 2021 via email

@yjs112233
Copy link
Author

is there only one instance of runtime/client active at each given time?
-- yes ! But there may be multiple instances at the same time.
eg:
Here is a file named A.torrent. My application will get a BtRuntime instance through the method .getBtRuntime(), create a BtClient with this BtRuntime instance, and start parsing and downloading the contents of A.torrent.
At the same time, B.torrent will get a new BtRuntime and a new BtClient in the same way, and start parsing and downloading the content of B.torrent.
C.torrent 、D.torrent、more and more....
Each BtRuntine is configured to allow automatic shutdown. When the download is complete, I confirm through the log that it has been closed,

End: After running some torrent files like this, and wait until all BtRuntime instance has been closed, A large number of connections were found in CLOSE_WAIT state. If more torrent files are run, the application will throw too many open files.

@pyckle
Copy link
Collaborator

pyckle commented Oct 6, 2021

However, after integration, I found that the same BT file cannot be received in the same BtRuntime, and it will stop running because it has been registered.

If this is the case, using the latest snapshot build should fix your issue.
This is the same bug as #146 and I believe this was fixed 1ad9d4c

Looking through the code briefly, I noticed the following

  • PeerConnectionPool does not close connections for torrents that are stopped - this seems like a bug. When a torrent is stopped, we should close all connections to peers of that torrent. When I find some time, I will try to fix this.
  • PeerConnectionPool creates an executor in its constructor that is never used. PR to fix this remove executor pool that was moved to ConnectionSource #193

Neither of these address your issue, but they probably should be fixed. I would suggest as a next step to better understand this issue

  • See if this reproduces on the latest code in github - many bugs have been fixed since 1.9
  • Enable trace logging in bt.net.SocketPeerConnection - see if close is called on the peers as the runtimes finish. If close is called, the bug is in the close code. if close is not called, then the bug is in the client shutdown behavior
  • Manually make your app run GC a few time (to ensure finalizers are called) via jcmd: https://docs.oracle.com/javase/8/docs/technotes/guides/troubleshoot/tooldescr006.html jcmd GC.run
  • If the GCs make these connections go away, it's definitely an issue with not calling close somewhere.

@yjs112233
Copy link
Author

thanks !

@yjs112233
Copy link
Author

Hi!I am back.
I downloaded the latest code and just added the code of #193, Now the application can work persistently.
It looks as if the leak has been fixed, The number of close_waits becomes normal.
On question #146, It does solve the problem of duplicate registrations.The fly in the ointment,The same TorrentId does not support running simultaneously in the same BtRuntime. If it does, BT will be better off.

@pyckle
Copy link
Collaborator

pyckle commented Oct 13, 2021 •
edited

Now the application can work persistently.
It looks as if the leak has been fixed, The number of close_waits becomes normal.

Excellent (:

The fly in the ointment,The same TorrentId does not support running simultaneously in the same BtRuntime. If it does, BT will be better off.

Can you explain why you have this use case? Why do you want to download a torrent multiple times in the same runtime?

From a protocol perspective, this can't be implemented because if an incoming peer connects to the torrent, the runtime won't know which instance to associate it with.

@yjs112233
Copy link
Author

yjs112233 commented Oct 13, 2021 •
edited

If BT is used as an service, two different people may want to download the same Torrent and it may happen at the same time.
If the protocol does not satisfy this idea, it may be implemented only through creating a new BtRuntime

@pyckle
Copy link
Collaborator

pyckle commented Oct 13, 2021

There are two solutions that I see for this use case.

  • Create a new runtime for each user. This is the easiest solution.
  • Use the same runtime, and if the torrent is already in the runtime, share the torrent between the two users, and delete it once no user wants it (reference counting).

The second solution is technically superior, but more coding, and some bt changes. If there are additional tracker(s) (announce/announce-list) in the torrent loaded second, those should be somehow added to the torrent which is downloading. Some of the work has been done for this PeerRegistry.addPeerSource(), but more work would need to be done to allow this API to be easily externally accessible.

Note, this second solution won't work properly with private trackers that track download/upload amounts because they often put a unique identifier in the announce url.

@yjs112233
Copy link
Author

yjs112233 commented Oct 13, 2021 •
edited

Thank you for helping me solve these lssues !

@pyckle
Copy link
Collaborator

pyckle commented Oct 13, 2021 •
edited

Sure 👍

@atomashpolskiy
Copy link
Owner

atomashpolskiy commented Oct 13, 2021 via email

@atomashpolskiy
Copy link
Owner

atomashpolskiy commented Oct 13, 2021 via email

@pyckle
Copy link
Collaborator

pyckle commented Oct 13, 2021

Another way would be to implement a storage that can be re-used by concurrent users without having to create a new BtClient instance.

Perhaps I misread the code, but I don't think that would work well, unless the download has finished. If the download hasn't finished, both runtimes will try and write the same unfinished blocks because they have no idea which blocks the other has downloaded, and when to has them. If there's content poisoning/a peer happens to send a corrupted chunk of data, one runtime could ban a good peer because the other runtime wrote a bad piece. Also, one runtime, unaware that data has been corrupted could send out bad data.

@atomashpolskiy
Copy link
Owner

atomashpolskiy commented Oct 13, 2021 via email

@atomashpolskiy
Copy link
Owner

atomashpolskiy commented Oct 13, 2021 via email

@yjs112233
Copy link
Author

yjs112233 commented Jan 28, 2022 •
edited

Another way would be to implement a storage that can be re-used by concurrent users without having to create a new BtClient instance.

Perhaps I misread the code, but I don't think that would work well, unless the download has finished. If the download hasn't finished, both runtimes will try and write the same unfinished blocks because they have no idea which blocks the other has downloaded, and when to has them. If there's content poisoning/a peer happens to send a corrupted chunk of data, one runtime could ban a good peer because the other runtime wrote a bad piece. Also, one runtime, unaware that data has been corrupted could send out bad data.

Perhaps my incomplete description misled you, I actually isolated the different BtRuntime downloads by setting different storage locations. So there is no block collision.
"The same TorrentId does not support running simultaneously in the same BtRuntime". On this question, I first want to know if sharing buckets between different clients is feasible.
If you want to use it as a business function outside of the BT protocol implementation,
Reference counting is a good way to do this.
If you want to be an extension of the BT protocol, I like the idea of using shared buckets. But really, it's also similar to reference counting.
I later changed my mind because this would result in a loss of data independence and privacy, It's important to stay privateIt's important to stay private, and since the issue arose in a business scenario, I preferred to meet this requirement at the business level rather than modify or extend the BT protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@pyckle @atomashpolskiy @yjs112233 and others