Multiple issues in capture()
Brought to you by:
pcharles
Menu
▾
▴
#47 Multiple issues in capture()
1. At the Java level, capture(int instance, int count) is unsafe and should not be public. Should be deprecated now and made private later.
The rest of this report concerns the JNI code.
2. endCapture() does not call pcap_breakloop(), which makes it necessary to add the same kind of support at the JNI level, with loops and booleans, which is pointless.
3. The pcap_loop() and pcap_dispatch() return codes of -2 are treated as errors, which they aren't. See the boldface comments on this matter in 'man libpcap'.
4. The pcap_dispatch() code loop is repeated in the source, to cover the cases where (i) DO_SELECT is in effect but capture is from a file, and (ii) DO_SELECT is not in operation. A simple rearrangement of the conditional compilation logic can remove this duplication.
5. The memory leak discussed in the forums is caused by 'new'-ing the C++ object and never deleting it. It should be allocated <i>on the stack,</i> via 'process p;' then, on any exit from the method, its destructor is automatically invoked, which can be useful, and the memory is freed. NB The memory leak is <b>not</b> caused by missing DeleteLocalRef() calls. The JNI documentation is quite clear that all local refs are freed when a JNI method returns. This API is only there if you want to free a ref earlier than that, e.g. in a long-running JNI method that allocates lots of objects. So in fact there are a number of these calls that can be <i>removed.</i>
6. A capture() count of zero is treated as infinity, same as -1. The Javadoc only says that -1 means infinity. Therefore zero should mean zero.
7. The expressions where 'count' is tested contain '? true :'. This is formally redundant and should be removed.
8. pcap_dispatch() is called with a 'count' of 1, which is inefficient at the lower level. This count should be the same as the count supplied by the application, and #2 above should be fixed at the same time. This makes the loop discussed at #8 above unnecessary in the non-select() case.
Logged In: YES
user_id=1262793
Originator: YES
Clarification. '? true :' can be replaced by '||' as a simple matter of formal logic.
Logged In: YES
user_id=1262793
Originator: YES
All this has been addressed in the forthcoming release 2.0.
About #5 -- The processing object is created at the very beginning of the capture method. In that case how can memory grow continuously due that object?
Do you think calling capture() in a loop from our application can solve the memory issue temporarily?
Regarding #5, the C++ object is new-ed every time you call capture(), and never deleted. The less often you call capture(), the slower the leak will be. So calling it in a Java loop is the worst thing you can do.
Thanks esmondpitt!! That's what I thought. However somebody suggested to call capture() in a loop in a forum thread.
In my case, I call capture() only once (infinity), but still the memory continues to leak. I agree that calling DeleteLocalRef inside processData() should not help (I actually tried by calling it on cls and jba, but no improvement). Do you know the reason? I am kinda stuck because of this.
There are other leaks. 2.0 should be out by the end of the year: development finished some time ago, we are just addressing build issues now.
esmondpitt, can you give me some clue about the reason of those leaks? I am supposed to release my product in this month. I want to put a temporary fix, compile jpcap and release the product and later I will upgrade to jpcap 2.0 once it is released. Thanks!!
esmondpitt, can you give me some clue about the reason of those leaks? I am supposed to release my product in this month. I want to put a temporary fix, compile jpcap and release the product and later I will upgrade to jpcap 2.0 once it is released. Thanks!!
The last release (v1.16) was 9/30/04. Is there a release date for v2.0? From looking at the web pages, it looks like this version of jpcap is all but dead.