search

SlideRuleEarth / sliderule

Server and client framework for on-demand science data processing in the cloud
https://slideruleearth.io
Other
27 stars 12 forks source link

Possible memory leaks in code creating threads when Thread constructor throws an exception. #416

Open elidwa opened 3 months ago

elidwa commented 3 months ago

The Thread constructor throws an exception if the pthread_create call fails. Review all code where Thread object is instantiated and ensure that there are no memory leaks.

For example, in the icsat2 plugin, all readers and the atl03 viewer catch the exception but do not free the info pointer resulting in memory leaks. The same issue is present in the GEDI and SWOT plugins. GeoIndexRaster lets the exception propagate all the way to luaSample call possibly leaking memory.

jpswinski commented 3 months ago

I'm not sure what the best approach is here... if we are unable to create a thread, we may want to just exit. But again, I am not sure. Being able to handle it would be great; but my worry is that handling the exception is just the tip of the iceberg. Is all of the downstream logic for each thread creation assuming the thread has been created and therefor potentially not safe? Not sure.

elidwa commented 3 months ago

There is another issue we need to address. The Atl03Reader code is a good example, but a similar problem exists in the raster code as well. If a reader needs to create, for example, 6 threads, and the last couple fail to be created, the code will catch the exception and execute the "no more data/signalComplete()" logic. However, the threads that were successfully created will continue running and sending data.

I am not sure if this is a problem or not but we should probably take a closer look at it. Ideally, if even one thread failed we should delete/join the ones already started and only than signalComplete() I am hoping this can be done in a safe way. I can instrument the code and create a test scenario.

elidwa commented 3 months ago

Raster code properly handles Thread creation exception. Code pushed on luaCreateCleanup branch, PR pending

jpswinski commented 3 months ago

Another idea is to create a "FatalException" class and have classes like the Thread class and the Timer class throw it when an exception occurs. It will be different than the "RunTimeException" which the developer should catch and gives some indication that giving up is acceptable.