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.

Sign up for GitHub

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

Jump to bottom

utils::chunked_vector fill constructor is exception unsafe #18635

Closed
bhalevy opened this issue May 13, 2024 · 0 comments · Fixed by #18636
Closed

utils::chunked_vector fill constructor is exception unsafe #18635

bhalevy opened this issue May 13, 2024 · 0 comments · Fixed by #18636
Assignees
@bhalevy
Labels
Backport candidate

Comments

@bhalevy
Copy link
Member

bhalevy commented May 13, 2024

If chunked_vector<T, max_contiguous_allocation>::chunked_vector(size_t n, const T& value) throws an exception,
the destructor won't be called, since the object is not fully constructed.
To make that happen, this constructor, like the other constructors, needs to first call the default constructor (that doesn't throw).

See

scylladb/utils/chunked_vector.hh

Line 354 in c8cc47d

chunked_vector<T, max_contiguous_allocation>::chunked_vector(size_t n, const T& value) {

@bhalevy bhalevy self-assigned this May 13, 2024
bhalevy added a commit to bhalevy/scylla that referenced this issue May 13, 2024
@bhalevy
utils: chunked_vector: fill ctor: make exception safe
07885c6 
Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not
fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes scylladb#18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
@bhalevy bhalevy mentioned this issue May 13, 2024
Merged
1 task
bhalevy added a commit to bhalevy/scylla that referenced this issue May 13, 2024
@bhalevy
chunked_vector_test: add test_chunked_vector_ctor_exception_safety
23f1bde 
Reproduces scylladb#18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
bhalevy added a commit to bhalevy/scylla that referenced this issue May 13, 2024
@bhalevy
chunked_vector_test: add more exception safety tests
4bbb66f 
For insertion, with and without reservation,
and for fill and copy constructors.

Reproduces scylladb#18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
denesb added a commit that referenced this issue May 14, 2024
@denesb
Merge 'utils: chunked_vector: fill ctor: make exception safe' from Be…
a15a9c3 
…nny Halevy

Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

- [x] Although the fixes is for a rare bug, it has very low risk and so it's worth backporting to all live versions

Closes #18636

* github.com:scylladb/scylladb:
  chunked_vector_test: add more exception safety tests
  chunked_vector_test: exception_safe_class: count also moved objects
  utils: chunked_vector: fill ctor: make exception safe
mergify bot pushed a commit that referenced this issue May 14, 2024
@bhalevy @mergify
utils: chunked_vector: fill ctor: make exception safe
b039012 
Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not
fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
(cherry picked from commit 64c51cf)
mergify bot pushed a commit that referenced this issue May 14, 2024
@bhalevy @mergify
chunked_vector_test: add more exception safety tests
3b35ea5 
For insertion, with and without reservation,
and for fill and copy constructors.

Reproduces #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
(cherry picked from commit 4bbb66f)

# Conflicts:
#	test/boost/chunked_vector_test.cc
mergify bot pushed a commit that referenced this issue May 14, 2024
@bhalevy @mergify
utils: chunked_vector: fill ctor: make exception safe
cabe50b 
Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not
fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
(cherry picked from commit 64c51cf)
mergify bot pushed a commit that referenced this issue May 14, 2024
@bhalevy @mergify
chunked_vector_test: add more exception safety tests
a7f817d 
For insertion, with and without reservation,
and for fill and copy constructors.

Reproduces #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
(cherry picked from commit 4bbb66f)

# Conflicts:
#	test/boost/chunked_vector_test.cc
This was referenced May 14, 2024
Merged
Merged
bhalevy added a commit that referenced this issue May 21, 2024
@bhalevy
utils: chunked_vector: fill ctor: make exception safe
8e20379 
Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not
fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
bhalevy added a commit that referenced this issue May 21, 2024
@bhalevy
chunked_vector_test: add more exception safety tests
36c66d5 
For insertion, with and without reservation,
and for fill and copy constructors.

Reproduces #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
bhalevy added a commit that referenced this issue May 21, 2024
@bhalevy
utils: chunked_vector: fill ctor: make exception safe
15a090f 
Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not
fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
bhalevy added a commit that referenced this issue May 21, 2024
@bhalevy
chunked_vector_test: add more exception safety tests
d947f1e 
For insertion, with and without reservation,
and for fill and copy constructors.

Reproduces #18635

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
denesb added a commit that referenced this issue May 21, 2024
@denesb
Merge '[Backport 5.4] utils: chunked_vector: fill ctor: make exceptio…
ae6c875 
…n safe' from ScyllaDB

Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

- [x] Although the fixes is for a rare bug, it has very low risk and so it's worth backporting to all live versions

(cherry picked from commit 64c51cf)

(cherry picked from commit 88b3173)

(cherry picked from commit 4bbb66f)

Refs #18636

Closes #18679

* github.com:scylladb/scylladb:
  chunked_vector_test: add more exception safety tests
  chunked_vector_test: exception_safe_class: count also moved objects
  utils: chunked_vector: fill ctor: make exception safe
denesb added a commit that referenced this issue May 21, 2024
@denesb
Merge '[Backport 5.2] utils: chunked_vector: fill ctor: make exceptio…
6f0d32a 
…n safe' from ScyllaDB

Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes #18635

- [x] Although the fixes is for a rare bug, it has very low risk and so it's worth backporting to all live versions

(cherry picked from commit 64c51cf)

(cherry picked from commit 88b3173)

(cherry picked from commit 4bbb66f)

 Refs #18636

Closes #18680

* github.com:scylladb/scylladb:
  chunked_vector_test: add more exception safety tests
  chunked_vector_test: exception_safe_class: count also moved objects
  utils: chunked_vector: fill ctor: make exception safe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bhalevy bhalevy

Labels
Backport candidate
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

utils: chunked_vector: fill ctor: make exception safe bhalevy/scylla
2 participants
@bhalevy @scylladb-promoter