Skip to content

Commit de7ebba

Browse files
author
Pan
committed
Added deallocation for sftp handles, tests.
1 parent 688b1b3 commit de7ebba

File tree

5 files changed

+55
-29
lines changed

5 files changed

+55
-29
lines changed

‎ssh/sftp.pxd‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ cimport c_sftp
2121

2222
cdef class SFTP:
2323
cdef c_sftp.sftp_session _sftp
24-
cdef Session session
24+
cdef readonly Session session
2525

2626
@staticmethod
2727
cdef SFTP from_ptr(c_sftp.sftp_session sftp, Session session)

‎ssh/sftp.pyx‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ from sftp_handles cimport SFTPFile, SFTPDir
2121
from sftp_attributes cimport SFTPAttributes
2222
from sftp_statvfs cimport SFTPStatVFS
2323
from utils cimport handle_ssh_error_codes, to_bytes
24-
from .exceptions import SFTPError
24+
from .exceptions import SFTPError, SFTPHandleError
2525

2626
cimport c_sftp
2727
from c_ssh cimport ssh_get_error, ssh_get_error_code, timeval
@@ -98,7 +98,7 @@ cdef class SFTP:
9898
with nogil:
9999
c_dir = c_sftp.sftp_opendir(self._sftp, c_path)
100100
if c_dir is NULL:
101-
raise SFTPError(ssh_get_error(self.session._session))
101+
raise SFTPHandleError(ssh_get_error(self.session._session))
102102
_dir = SFTPDir.from_ptr(c_dir, self)
103103
return _dir
104104

@@ -134,7 +134,7 @@ cdef class SFTP:
134134
with nogil:
135135
c_file = c_sftp.sftp_open(self._sftp, c_path, accesstype, mode)
136136
if c_file is NULL:
137-
raise SFTPError(ssh_get_error(self.session._session))
137+
raise SFTPHandleError(ssh_get_error(self.session._session))
138138
_file = SFTPFile.from_ptr(c_file, self)
139139
return _file
140140

@@ -144,15 +144,19 @@ cdef class SFTP:
144144
cdef int rc
145145
with nogil:
146146
rc = c_sftp.sftp_unlink(self._sftp, c_path)
147-
return handle_ssh_error_codes(rc, self.session._session)
147+
if rc < 0:
148+
raise SFTPError(ssh_get_error(self.session._session))
149+
return rc
148150

149151
def rmdir(self, path not None):
150152
cdef bytes b_path = to_bytes(path)
151153
cdef const char *c_path = b_path
152154
cdef int rc
153155
with nogil:
154156
rc = c_sftp.sftp_rmdir(self._sftp, c_path)
155-
return handle_ssh_error_codes(rc, self.session._session)
157+
if rc < 0:
158+
raise SFTPError(ssh_get_error(self.session._session))
159+
return rc
156160

157161
def mkdir(self, path not None, c_sftp.mode_t mode):
158162
cdef bytes b_path = to_bytes(path)
@@ -170,7 +174,9 @@ cdef class SFTP:
170174
cdef int rc
171175
with nogil:
172176
rc = c_sftp.sftp_rename(self._sftp, c_orig, c_newname)
173-
return handle_ssh_error_codes(rc, self.session._session)
177+
if rc < 0:
178+
raise SFTPError(ssh_get_error(self.session._session))
179+
return rc
174180

175181
def setstat(self, path not None, SFTPAttributes attr):
176182
cdef bytes b_path = to_bytes(path)
@@ -187,15 +193,19 @@ cdef class SFTP:
187193
cdef int rc
188194
with nogil:
189195
rc = c_sftp.sftp_chown(self._sftp, c_path, owner, group)
190-
return handle_ssh_error_codes(rc, self.session._session)
196+
if rc < 0:
197+
raise SFTPError(ssh_get_error(self.session._session))
198+
return rc
191199

192200
def chmod(self, path not None, c_sftp.mode_t mode):
193201
cdef bytes b_path = to_bytes(path)
194202
cdef const char *c_path = b_path
195203
cdef int rc
196204
with nogil:
197205
rc = c_sftp.sftp_chmod(self._sftp, c_path, mode)
198-
return handle_ssh_error_codes(rc, self.session._session)
206+
if rc < 0:
207+
raise SFTPError(ssh_get_error(self.session._session))
208+
return rc
199209

200210
def utimes(self, path not None, long seconds, long microseconds):
201211
cdef bytes b_path = to_bytes(path)
@@ -211,7 +221,9 @@ cdef class SFTP:
211221
_val.tv_usec = microseconds
212222
rc = c_sftp.sftp_utimes(self._sftp, c_path, _val)
213223
free(_val)
214-
return handle_ssh_error_codes(rc, self.session._session)
224+
if rc < 0:
225+
raise SFTPError(ssh_get_error(self.session._session))
226+
return rc
215227

216228
def symlink(self, source not None, dest not None):
217229
cdef bytes b_source = to_bytes(source)
@@ -221,7 +233,9 @@ cdef class SFTP:
221233
cdef int rc
222234
with nogil:
223235
rc = c_sftp.sftp_symlink(self._sftp, c_source, c_dest)
224-
return handle_ssh_error_codes(rc, self.session._session)
236+
if rc < 0:
237+
raise SFTPError(ssh_get_error(self.session._session))
238+
return rc
225239

226240
def readlink(self, path not None):
227241
cdef bytes b_path = to_bytes(path)
@@ -231,9 +245,7 @@ cdef class SFTP:
231245
with nogil:
232246
_link = c_sftp.sftp_readlink(self._sftp, c_path)
233247
if _link is NULL:
234-
return handle_ssh_error_codes(
235-
ssh_get_error_code(self.session._session),
236-
self.session._session)
248+
raise SFTPError(ssh_get_error(self.session._session))
237249
b_link = _link
238250
return b_link
239251

@@ -245,9 +257,7 @@ cdef class SFTP:
245257
with nogil:
246258
c_vfs = c_sftp.sftp_statvfs(self._sftp, c_path)
247259
if c_vfs is NULL:
248-
return handle_ssh_error_codes(
249-
ssh_get_error_code(self.session._session),
250-
self.session._session)
260+
raise SFTPError(ssh_get_error(self.session._session))
251261
vfs = SFTPStatVFS.from_ptr(c_vfs, self)
252262
return vfs
253263

@@ -259,14 +269,12 @@ cdef class SFTP:
259269
with nogil:
260270
_rpath = c_sftp.sftp_canonicalize_path(self._sftp, c_path)
261271
if _rpath is NULL:
262-
return handle_ssh_error_codes(
263-
ssh_get_error_code(self.session._session),
264-
self.session._session)
272+
raise SFTPError(ssh_get_error(self.session._session))
265273
b_rpath = _rpath
266274
return b_rpath
267275

268276
def server_version(self):
269277
cdef int rc
270278
with nogil:
271279
rc = c_sftp.sftp_server_version(self._sftp)
272-
return rc
280+
return handle_ssh_error_codes(rc, self.session._session)

‎ssh/sftp_handles.pxd‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ cimport c_sftp
2222

2323
cdef class SFTPFile:
2424
cdef c_sftp.sftp_file _file
25-
cdef SFTP sftp
25+
cdef readonly SFTP sftp
2626
cdef readonly bint closed
2727

2828
@staticmethod
@@ -31,7 +31,8 @@ cdef class SFTPFile:
3131

3232
cdef class SFTPDir:
3333
cdef c_sftp.sftp_dir _dir
34-
cdef SFTP sftp
34+
cdef readonly SFTP sftp
35+
cdef readonly bint closed
3536

3637
@staticmethod
3738
cdef SFTPDir from_ptr(c_sftp.sftp_dir _dir, SFTP sftp)

‎ssh/sftp_handles.pyx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ cdef class SFTPFile:
2929

3030
def __cinit__(self, SFTP sftp):
3131
self.sftp = sftp
32-
self.closed = False
3332

3433
def __dealloc__(self):
35-
pass
34+
if not self.closed:
35+
self.close()
3636

3737
@staticmethod
3838
cdef SFTPFile from_ptr(c_sftp.sftp_file _file, SFTP sftp):
@@ -55,10 +55,6 @@ cdef class SFTPFile:
5555
return size, data
5656
raise StopIteration
5757

58-
@property
59-
def sftp_session(self):
60-
return self.sftp
61-
6258
def fstat(self):
6359
cdef SFTPAttributes _attrs
6460
cdef c_sftp.sftp_attributes c_attrs
@@ -205,6 +201,10 @@ cdef class SFTPDir:
205201
def __cinit__(self, SFTP sftp):
206202
self.sftp = sftp
207203

204+
def __dealloc__(self):
205+
if not self.closed:
206+
self.closedir()
207+
208208
@staticmethod
209209
cdef SFTPDir from_ptr(c_sftp.sftp_dir _dir, SFTP sftp):
210210
cdef SFTPDir _fh = SFTPDir.__new__(SFTPDir, sftp)
@@ -239,8 +239,13 @@ cdef class SFTPDir:
239239

240240
def closedir(self):
241241
cdef int rc
242+
if self.closed:
243+
return 0
242244
with nogil:
243245
rc = c_sftp.sftp_closedir(self._dir)
246+
if rc < 0:
247+
raise SFTPHandleError(ssh_get_error(self.sftp.session._session))
248+
self.closed = True
244249
return rc
245250

246251
cpdef SFTPAttributes readdir(self):

‎tests/test_sftp.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,14 @@ def test_sftp_readdir(self):
8181
self.assertTrue(attrs.size > 0)
8282
self.assertEqual(attrs.name, b'.')
8383
self.assertTrue(len(attrs.longname) > 1)
84+
self.assertFalse(_dir.closed)
85+
self.assertEqual(_dir.closedir(), 0)
86+
self.assertTrue(_dir.closed)
87+
del _dir
88+
with sftp.opendir('.') as _dir:
89+
pass
90+
self.assertTrue(_dir.closed)
91+
del _dir
8492

8593
def test_sftp_file_read(self):
8694
self._auth()
@@ -105,6 +113,10 @@ def test_sftp_file_read(self):
105113
self.assertTrue(remote_fh.closed)
106114
self.assertEqual(remote_data, test_file_data)
107115
self.assertTrue(remote_fh.closed)
116+
del remote_fh
117+
with sftp.open(remote_filename, os.O_RDONLY, 0) as remote_fh:
118+
pass
119+
self.assertTrue(remote_fh.closed)
108120
finally:
109121
os.unlink(remote_filename)
110122

0 commit comments

Comments
 (0)