Fixes for transient NullPointerExceptions

joval commented 12 years ago

Greetings,

I've started using JDBM in my jOVAL project, and have experienced some NullPointerExceptions that I am guessing must be the result of race conditions because they don't seem to happen very consistently. Anyway, for some reason my diffs look bad on Github, so here are the whitespace-ignoring diffs (below). The change to SerialClassInfo was inadvertent -- I just deleted a blank line after having temporarily inserted a printStackTrace statement.

One comment I would make is that it would be nice if the JDBM maps had the ability to detect when changes are made to a previously inserted object (without having to re-add it), so that changes would be serialized transparently -- in autocommit mode, anyway. This would square its behavior with the in-memory Hashtable class, for instance. This was causing me some grief that took a long time to figure out!

Please let me know if you have any questions or comments.

Best regards, --David Solin

$ git diff -w 98b794ccc4e2a73a955c62e0601f43f44dd139ca.. diff --git a/src/main/java/org/apache/jdbm/DBAbstract.java b/src/main/java/org/apache/jdbm/DBAbstract.java index ca5e1ae..140f0b1 100644 --- a/src/main/java/org/apache/jdbm/DBAbstract.java +++ b/src/main/java/org/apache/jdbm/DBAbstract.java @@ -398,6 +398,9 @@ abstract class DBAbstract implements DB { if(nameDirectory_recid==0) return ret; HTree<String,Long> m = fetch(nameDirectory_recid);

if (m == null) {
return null;
```
    }

   for(Map.Entry<String,Long> e:m.entrySet()){
       Object o = fetch(e.getValue());
```
diff --git a/src/main/java/org/apache/jdbm/HTreeDirectory.java b/src/main/java/org/apache/jdbm/HTreeDirectory.java index 50cb20e..e882b4d 100644 --- a/src/main/java/org/apache/jdbm/HTreeDirectory.java +++ b/src/main/java/org/apache/jdbm/HTreeDirectory.java @@ -154,6 +154,9 @@ final class HTreeDirectory<K, V> { HTreeDirectory<K, V> dir = (HTreeDirectory<K, V>) node; dir.setPersistenceContext(child_recid); return dir.get(key);
} else if (node == null) {
putRecid(hash, 0L);
```
        return null;
   } else {
       // node is a bucket
       HTreeBucket<K, V> bucket = (HTreeBucket) node;
```
diff --git a/src/main/java/org/apache/jdbm/SerialClassInfo.java b/src/main/java/org/apache/jdbm/SerialClassInfo.java index c586412..595576f 100644 --- a/src/main/java/org/apache/jdbm/SerialClassInfo.java +++ b/src/main/java/org/apache/jdbm/SerialClassInfo.java @@ -349,7 +349,6 @@ abstract class SerialClassInfo { return f.get(object); }

} catch (Exception e) {
```
      }

      throw new NoSuchFieldError(object.getClass() + "." + fieldInfo.getName());
```

jankotek commented 12 years ago

Hi,

I dugout patch from your fork. Basically it prevents NPE in HashMap. I am afraid that this would just mask deeper problem, HashMap does not seems to be thread-safe.

I believe it is better to fail, then save/return incorrect data. So I am not going to integrate this patch. If you could help me to reproduce NPE we could probably fix original race condition.

jankotek commented 12 years ago

Reopening, patch was refused, but null pointers are still not fixed

joval commented 12 years ago

Hi Jan,

Thanks for looking! I'll check out your change in the next few days and see if it solves my problem. If not I'll let you know and we can dig deeper.

Regards, --David

On 8/21/2012 3:51 PM, Jan Kotek wrote:

Reopening, patch was refused, but null pointers are still not fixed

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-7915705.

jOVAL.org: OVAL implemented in Java. /Scan any machine from any machine. For free!/ Learn More http://www.joval.org | Features http://www.joval.org/features/ | Download http://www.joval.org/download/

joval commented 12 years ago

Hi Jan,

I've merged your changes and did a preliminary smoke test and they appear to have done the trick. I just need to put it through its paces in the test framework and then I'll let you know if you can close the issue out for good.

Thanks! --David

On 8/21/2012 3:46 PM, Jan Kotek wrote:

Hi,

I dugout patch from your fork. Basically it prevents NPE in HashMap. I am afraid that this would just mask deeper problem, HashMap does not seems to be thread-safe.

I believe it is better to fail, then save/return incorrect data. So I am not going to integrate this patch. If you could help me to reproduce NPE we could probably fix original race condition.

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-7915532.

jOVAL.org: OVAL implemented in Java. /Scan any machine from any machine. For free!/ Learn More http://www.joval.org | Features http://www.joval.org/features/ | Download http://www.joval.org/download/

jankotek commented 12 years ago

I think there are two different concurrent problems

1) named collections in DB (this confirmed and now fixed) 2) something within HashTree. I need to investigate it more.

So please keep running your test. Stacktrace with NPE inside HTree could really help me.

Jan

On 23/08/12 23:00, jOVAL.org wrote:

Hi Jan,

I've merged your changes and did a preliminary smoke test and they appear to have done the trick. I just need to put it through its paces in the test framework and then I'll let you know if you can close the issue out for good.

Thanks! --David

On 8/21/2012 3:46 PM, Jan Kotek wrote:

Hi,

I dugout patch from your fork. Basically it prevents NPE in HashMap. I am afraid that this would just mask deeper problem, HashMap does not seems to be thread-safe.

I believe it is better to fail, then save/return incorrect data. So I am not going to integrate this patch. If you could help me to reproduce NPE we could probably fix original race condition.

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-7915532.

jOVAL.org: OVAL implemented in Java. /Scan any machine from any machine. For free!/ Learn More http://www.joval.org | Features http://www.joval.org/features/ | Download http://www.joval.org/download/

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-7985705.

joval commented 12 years ago

Hi Jan,

I ran my full test suite and there were no exceptions; JDBM worked great! This is the class where I use the DB and TreeHash classes: https://github.com/joval/jOVAL/blob/master/src/org/joval/util/CachingHierarchy.java

This class has two functions: (1) to store information about a hierarchy as it's accessed so that repeated calls can return quickly, and (2) to create information about a hierarchy all at once, and then to serve as a backing store for subsequent queries against the hierarchy. An example of #2 is, we use a find command on a remote Unix host to list information about every file on disk, and create a local TreeHash indexed by path, so that multiple regular-expression-based searches can be executed against the TreeHash without incurring additional network traffic or load on the target server. Before JDBM, I had been using a regular Java Hashtable but these were getting very large in memory (~1GB, to store metadata for ~300K files). By storing the data in a JDBM TreeHash, it's possible to scan multiple remote machines simultaneously without using an ungodly amount lot of memory, and without taking too much of a performance hit.

Let me know if you make any other changes that you'd like me to test out.

Best regards, --David Solin

On 8/24/2012 11:43 AM, Jan Kotek wrote:

I think there are two different concurrent problems

1) named collections in DB (this confirmed and now fixed) 2) something within HashTree. I need to investigate it more.

So please keep running your test. Stacktrace with NPE inside HTree could really help me.

Jan

Jan

On 23/08/12 23:00, jOVAL.org wrote:

Hi Jan,

I've merged your changes and did a preliminary smoke test and they appear to have done the trick. I just need to put it through its paces in the test framework and then I'll let you know if you can close the issue out for good.

Thanks! --David

On 8/21/2012 3:46 PM, Jan Kotek wrote:

Hi,

I dugout patch from your fork. Basically it prevents NPE in HashMap. I am afraid that this would just mask deeper problem, HashMap does not seems to be thread-safe.

I believe it is better to fail, then save/return incorrect data. So I am not going to integrate this patch. If you could help me to reproduce NPE we could probably fix original race condition.

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-7915532.

jOVAL.org: OVAL implemented in Java. /Scan any machine from any machine. For free!/ Learn More http://www.joval.org | Features http://www.joval.org/features/ | Download http://www.joval.org/download/

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-7985705.

— Reply to this email directly or view it on GitHub https://github.com/jankotek/JDBM3/pull/87#issuecomment-8007916.

jOVAL.org: OVAL implemented in Java. /Scan any machine from any machine. For free!/ Learn More http://www.joval.org | Features http://www.joval.org/features/ | Download http://www.joval.org/download/

jankotek / JDBM3

Fixes for transient NullPointerExceptions #87

} catch (Exception e) {