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

Missing Gdal dataset open flags #154

Closed
dmarteau opened this issue Jan 29, 2021 · 6 comments · Fixed by #155
Closed

Missing Gdal dataset open flags #154

dmarteau opened this issue Jan 29, 2021 · 6 comments · Fixed by #155

Comments

@dmarteau
Copy link
Contributor

The gdal::open_ex open_flags allow only value from the GDALAccess::Type values.

This prevent using the others GDAL_OF_XXX flags combination.

They are preprocessor #define in gdal.h and thus skipped by bindgen; they are still essential to control some opening behaviors like raster/vector driver selection, or multidimensional raster or gnm drivers.

Maybe a crate like bitflags could help.

dmarteau added a commit to dmarteau/gdal that referenced this issue Jan 30, 2021
dmarteau added a commit to dmarteau/gdal that referenced this issue Jan 30, 2021
@rmanoka
Copy link
Contributor

rmanoka commented Jan 30, 2021

The bitflags approach is very cool; I'd opened a similar issue but this is more comprehensive, so I'm closing it. There is one issue with the open_ex options: GDAL_OF_SHARED that needs to be discussed (see below).

Sharing Issue

GDAL since 2.4 allows Dataset to be accessed from different threads, as long as only one thread accesses it at any time (ref. discussion). In this crate, we impl Send on Dataset to reflect that we can pass it to another thread, but not Sync so it is not accessed from two threads at the same time.

However, the GDAL_OF_SHARED option allows this to be subverted. We might open the same dataset twice with the shared option, and then pass one of the Dataset handles to another thread. This could lead to undefined behaviours as we are accessing a non-thread-safe data-structure from different threads simultaneously.

The simplest solution I could think of is to remove GDAL_OF_SHARED from the bitflags, thus not allowing it in the flags. Then, we could add a flag in DatasetOptions which can only be set via an unsafe method. This way, we allow shared open, if the user knows what they're doing, and there's no way to get UB via safe rust.

@jdroenner
Copy link
Member

wow this is a really cool solution. I think @rmanoka is right. we should remove the shared bitfag and add an unsafe method to DatasetOptions to enable this feature.

@dmarteau
Copy link
Contributor Author

I do agree that GDAL_OF_SHARED defeat the purpose of multithreading safety and I'm also in favor te remove it.

@dmarteau
Copy link
Contributor Author

Then, we could add a flag in DatasetOptions which can only be set via an unsafe method

Amha, this is even not necessary and it would be better to rely on rust safe way of handling shared access to dataset.

dmarteau added a commit to dmarteau/gdal that referenced this issue Jan 30, 2021
@jdroenner
Copy link
Member

i guess we can add a special method for shared later ( if we need it)

@rmanoka
Copy link
Contributor

rmanoka commented Jan 30, 2021

Amha, this is even not necessary and it would be better to rely on rust safe way of handling shared access to dataset.

Sounds good; bitflags already has an unsafe fn from_bits_unchecked that allows this anyway.

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

Successfully merging a pull request may close this issue.

3 participants