Skip to content

Commit f5289a1

Browse files
committed
common/ompio: store correctly the SHAREDFP_IS_SET flag
it looks like disabling the lazy_open flag for sharedfp components revealead a bug that lead to a crash in file_close in some tests. Make sure the SHAREDFP_IS_SET flag is correctly set (and not overwritten again), and we use that to avoid a double-free of the communicator. Signed-off-by: Edgar Gabriel <[email protected]>
1 parent c6595c2 commit f5289a1

File tree

2 files changed

+32
-32
lines changed

2 files changed

+32
-32
lines changed

ompi/mca/common/ompio/common_ompio_file_open.c

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ int mca_common_ompio_file_open (ompi_communicator_t *comm,
8282
/* No need to duplicate the communicator if the file_open is called
8383
from the sharedfp component, since the comm used as an input
8484
is already a dup of the user level comm. */
85-
ompio_fh->f_flags |= OMPIO_SHAREDFP_IS_SET;
8685
ompio_fh->f_comm = comm;
8786
}
8887

@@ -149,26 +148,9 @@ int mca_common_ompio_file_open (ompi_communicator_t *comm,
149148
** function will return an error code.
150149
*/
151150
}
152-
153-
/* open the file once more for the shared file pointer if required.
154-
** Can be disabled by the user if no shared file pointer operations
155-
** are used by his application.
156-
*/
157-
if ( NULL != ompio_fh->f_sharedfp &&
158-
true == use_sharedfp &&
159-
(!mca_io_ompio_sharedfp_lazy_open ||
160-
!strcmp (ompio_fh->f_sharedfp_component->mca_component_name,
161-
"addproc") )) {
162-
ret = ompio_fh->f_sharedfp->sharedfp_file_open(comm,
163-
filename,
164-
amode,
165-
info,
166-
ompio_fh);
167-
168-
if ( OMPI_SUCCESS != ret ) {
169-
goto fn_fail;
170-
}
171-
}
151+
}
152+
else {
153+
ompio_fh->f_flags |= OMPIO_SHAREDFP_IS_SET;
172154
}
173155

174156
/*Determine topology information if set*/
@@ -185,15 +167,32 @@ int mca_common_ompio_file_open (ompi_communicator_t *comm,
185167
info,
186168
ompio_fh);
187169

188-
189-
190-
191170
if ( OMPI_SUCCESS != ret ) {
192171
ret = MPI_ERR_FILE;
193172
goto fn_fail;
194173
}
195174

196175

176+
if ( true == use_sharedfp ) {
177+
/* open the file once more for the shared file pointer if required.
178+
** Can be disabled by the user if no shared file pointer operations
179+
** are used by his application.
180+
*/
181+
if ( NULL != ompio_fh->f_sharedfp &&
182+
!mca_io_ompio_sharedfp_lazy_open ) {
183+
ret = ompio_fh->f_sharedfp->sharedfp_file_open(comm,
184+
filename,
185+
amode,
186+
info,
187+
ompio_fh);
188+
189+
if ( OMPI_SUCCESS != ret ) {
190+
goto fn_fail;
191+
}
192+
}
193+
}
194+
195+
197196
/* If file has been opened in the append mode, move the internal
198197
file pointer of OMPIO to the very end of the file. */
199198
if ( ompio_fh->f_amode & MPI_MODE_APPEND ) {
@@ -205,16 +204,13 @@ int mca_common_ompio_file_open (ompi_communicator_t *comm,
205204
mca_common_ompio_set_explicit_offset (ompio_fh, current_size);
206205
if ( true == use_sharedfp ) {
207206
if ( NULL != ompio_fh->f_sharedfp &&
208-
(!mca_io_ompio_sharedfp_lazy_open ||
209-
!strcmp (ompio_fh->f_sharedfp_component->mca_component_name,
210-
"addproc") )) {
211-
207+
!mca_io_ompio_sharedfp_lazy_open ) {
212208
shared_fp_base_module = ompio_fh->f_sharedfp;
213209
ret = shared_fp_base_module->sharedfp_seek(ompio_fh,current_size, MPI_SEEK_SET);
214210
}
215211
else {
216212
opal_output(1, "mca_common_ompio_file_open: Could not adjust position of "
217-
"shared file pointer whith MPI_MODE_APPEND\n");
213+
"shared file pointer with MPI_MODE_APPEND\n");
218214
ret = MPI_ERR_OTHER;
219215
goto fn_fail;
220216
}
@@ -362,7 +358,7 @@ int mca_common_ompio_file_close (mca_io_ompio_file_t *ompio_fh)
362358
}
363359

364360

365-
if (MPI_COMM_NULL != ompio_fh->f_comm) {
361+
if (MPI_COMM_NULL != ompio_fh->f_comm && !(ompio_fh->f_flags & OMPIO_SHAREDFP_IS_SET) ) {
366362
ompi_comm_free (&ompio_fh->f_comm);
367363
}
368364

ompi/mca/common/ompio/common_ompio_file_view.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,12 @@ int mca_common_ompio_set_view (mca_io_ompio_file_t *fh,
8686
}
8787

8888
/* Reset the flags first */
89-
fh->f_flags = 0;
90-
89+
if ( fh->f_flags & OMPIO_CONTIGUOUS_FVIEW ) {
90+
fh->f_flags &= ~OMPIO_CONTIGUOUS_FVIEW;
91+
}
92+
if ( fh->f_flags & OMPIO_UNIFORM_FVIEW ) {
93+
fh->f_flags &= ~OMPIO_UNIFORM_FVIEW;
94+
}
9195
fh->f_flags |= OMPIO_FILE_VIEW_IS_SET;
9296
fh->f_datarep = strdup (datarep);
9397
datatype_duplicate (filetype, &fh->f_orig_filetype );

0 commit comments

Comments
 (0)