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.

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

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

Open
elidwa opened this issue Jun 18, 2024 · 4 comments
Assignees

Comments

@elidwa
Copy link
Contributor

elidwa commented Jun 18, 2024

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.

@elidwa elidwa self-assigned this Jun 18, 2024
@jpswinski
Copy link
Member

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
Copy link
Contributor Author

elidwa commented Jun 19, 2024

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
Copy link
Contributor Author

elidwa commented Jun 20, 2024

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

@jpswinski
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants