crashes observed when replacing attributes and changing their types

[jtbraun]

Attached are two test cases, one written in python against python's netCDF4 module (which was compiled against Ubuntu 14.04's 1:4.1.3-7ubuntu2 versions of the netcdf packages as noted), and a C version of the same test case, which was tested against the "ubuntu" version of libnetcdf as well as the latest version from github.

The root of the problem:
If you set an attribute using one of nc_put_att_string or nc_put_att_text, and then use the other function to set it to a different value, depending on the library and the order sometimes a segfault will occur. One version (test case 6) appears to have been fixed in the github library, and is an an invalid free() call in nc4_att_list_del(). Test case 7 is a crash in HDF5, possibly for similar reasons.

If the attribute is deleted using nc_del_att() before setting it the second time, no crashes occur with either test case, either library. Perhaps nc_put_att* doesn't do enough cleanup or bookkeeping when changing the type of the attribute?

[jtbraun]

Huh, I can't attach files that I can see, so here's a gist with the two errors:
https://gist.github.com/jtbraun/5e3db389a7e0b3b7fe8e

[WardF]

Thanks for the gist; I'll take a look at this now, thanks!

[durack1]

@WardF I wonder if this bug should be set against the 4.4.0 milestone? It'd be great to get this fixed for the upcoming release..

[WardF]

@durack1 It would be nice; I'm getting caught up having just returned from AGU15. I'm currently trying to fix a different issue, #159, after which I will evaluate the existing issues (including this one) and see what we can get done without further delaying the 4.4.0 release. I agree that this would be great to have fixed for that milestone.

[durack1]

@WardF we have a very simple test case that I can run to provide fairly prompt feedback, so do let me know.. It'd be great to fix this issue as segfaults aren't the best behavior..

[WardF]

I agree completely; this should be fixed ASAP. We have another attribute renaming issue that Russ Rew was in the middle of fixing when he retired; I'm coordinating with him to get the information regarding what he had completed. I'm wondering if this is related.

To make sure that I'm reading the test case (repro.c) correctly, the first 5 test cases pass, 6 is fixed in the master branch and 7 still occurs but you think might be to do with hdf5, correct? I will create a branch and add this test but I want to make sure I'm on the same page.

I agree that segfaults aren't the best behavior; however, I'm even more worried by issues like #159, where incorrect data might be silently written (under very specific circumstances on a specific platform). Nevertheless, all issues will be fixed :).

[WardF]

I've started digging into this and it immediately felt familiar. This is supported by the fact that it appears to be fixed in the development branch and, scanning closed issues, I think it was captured/fixed as part of #149, which I think Jeff Whitaker opened in response to your original reporting of this issue.

In any event, the test you provided passes in the development branch, which means it should roll out as part of the 4.4.0 release. Closing this issue, but I will reopen if you find that you still have a test case where it continues to fail.