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

Fix unclosed socket in TraCI Connection initializer #10670

Merged
merged 2 commits into from May 4, 2022

Conversation

Gamenot
Copy link
Contributor

@Gamenot Gamenot commented May 4, 2022

Solution

I propose a minor fix to eliminate a resource warning originating from the traci.Connection initializer.

The second client example in the socket Python library documentation indicates that socket.connect() may not clean itself up if it generates an exception: https://docs.python.org/3.5/library/socket.html#example

# Echo client program
...
    try:
        s.connect(sa)
    except OSError as msg:
        s.close()
        s = None
        continue
    break
...

Impact

Using traci.connect() would cause an unclosed socket to get garbage collected and give a resource warning. The error is known about and caught at the traci.connect() level without clean-up of the socket.

https://github.com/eclipse/sumo/blob/49ba1525d34ae23b515d08cf2643975a1f7b169c/tools/traci/main.py#L89-L111

It was causing warnings to show up and mess with logging, requiring a workaround. A stranger quickly reached out to make it known that the issue was not unique to us: huawei-noah/SMARTS#1373

self._socket.connect((host, port))
try:
self._socket.connect((host, port))
except socket.error as e:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have kept the socket.error alias of OSError for consistency's sake.

tools/traci/connection.py Outdated Show resolved Hide resolved
Co-authored-by: Tucker Alban <mtuck2005@yahoo.ca>
@behrisch behrisch merged commit 0ba2048 into eclipse-sumo:main May 4, 2022
@behrisch
Copy link
Contributor

behrisch commented May 4, 2022

Thanks!

behrisch added a commit that referenced this pull request May 9, 2022
* fix #10588

* fix #10591

* Added updateDailyVersion.py. Refs #10086

* Fixed #10086

* Added documentation for aggregateBatteryOutput. Refs #2

* Changelog. Refs #2

* adapting test #10589 #21

* fix #10595

* Now saveAs... buttons are only enabled if there is elements to save. Refs #10426

* fix #10582

* patching expected results refs #21, #10582

* Separator is hidden in supermode Data. Refs #10426

* Revert fix for #10582: Sometimes empty street names are intentional (i.e. on motorways that have a bridge)

* Updated EditMenuCommands. Refs #10426

* Updated NETEDIT test. Refs #3045

* adding test refs #10597, #21

* activating more meso test refs #21

* fix #10600

* patching meso results refs #21, #10600

* NEMA Controller Re-write + SUMO UC 2022 Fixes / Tweaks (#10598)

* Adds the NEMA controller in FSM fashion

Signed-off-by: Max Schrader <mcschrader@crimson.ua.edu>

* Updates test files to specify Type170 controller

Signed-off-by: Max Schrader <mcschrader@crimson.ua.edu>

* Updates test results to match new NEMA

Signed-off-by: Max Schrader <mcschrader@crimson.ua.edu>

* Updated NEMA documentation to match state of controller

Signed-off-by: Max Schrader <mcschrader@crimson.ua.edu>

* patching expected results refs #21, #10598

* correcting error message time formatting from steps (ms) to seconds #10598

* patching expected results refs #21, #10598

* #2 fixing internal link

* added test refs #10599, #21

* fix #10599

* added test refs #10603, #21

* fix #10603

* patching expected results refs #21, #10603

* fix #10582

* patching expected results refs #21, #10582

* fixing warnings #3 #10595

* improving emergency braking warning #10604 fixing more warnings

* adapting tests to emergency braking warning #21 fix #10604

* adding map providers #9787 fix #10506

* patching expected results refs #21, #10582

* Updated NETEDIT test. Refs #3045

* Added new reload options. Refs #10426

* Minor update in GNEApplicationWindow. Refs #10426

* Fixed #10426

* fix #10606

* patching expected results refs #21, #10606

* Updated NETEDIT test. Refs #3045

* Updated NETEDIT test. Refs #3045

* added test refs #10610, #21

* fix #106106

* added test refs #10612, #21

* fix #10612

* Updated NETEDIT test. Refs #3045

* fix #10613

* patching expected results refs #21, #10613

* adapting meso tests #21

* enabling more tests #10602

* added test refs #10614, #21

* fix #10614

* patching expected results refs #21, #10604

* added tests refs #10615, #21

* fix #10615

* Updated NETEDIT test. Refs #3045

* Updated NETEDIT test. Refs #3045

* fixing style #22

* patching expected results refs #21, #10604

* patching expected results refs #21, #10582

* #2 updated deprecated method name

* testing whether extra tests do run at all #21

* compatibility with stone age compilers #3

* Fixed #10622 and fixed #10588

* Added new NETEDIT test. Refs #3045, #10588 #10591

* tweaking debug code

* added test refs #10626, #21

* added test refs #10625, #21

* Fixed #10627

* refactoring refs #12

* fix #10625

* patching expected results refs #21, #10625

* added test refs #10628, #21

* fix #10628

* patching expected results refs #21, #10628

* addendum refs #10628

* patching expected results refs #21, #10628

* Added new NETEDIT test. Refs #3045

* Updated NETEDIT tests. Refs #3045

* Updated NETEDIT tests. Refs #3045

* Fixed problem loading trips and flow over junctions. Refs #457

* Updated NETEDIT tests. Refs #3045

* Updated NETEDIT tests. Refs #3045

* added emission output test refs #21

* added test refs #10632, #21

* added test refs #10632, #21

* refactoring refs #12

* storing routeID for walk refs #10632

* added test refs #10632, #21

* fix #10632

* fix #10633

* fix #10629

* fix #10635

* #2 Changelog

* fix #10608

* fix #10636 (default appearance unchanged)

* adding failing test #21 #10637

* fix #10634

* Updated aggregateBatteryOutput.py. Refs #10609

* Updated aggregateBatteryOutput.py. Refs #10609

* Updated aggregateBatteryOutput.py. Refs #10609

* Updated aggregateBatteryOutput.py. Refs #10609

* Fixed #10609

* Added new test. Refs #10609

* fix #10637

* patching expected results refs #21, #10637

* tweaking warning refs #8992

* patching expected results refs #21, #8992

* #2 minor wording changes

* patching expected results refs #21, #10637

* tweaking debug code

* added test refs #10641, #21

* fix #10641

* patching expected results refs #21, #10641

* removing plot output refs #21

* yield to dangerous (junction) foes when stuck on junction after green phase ends. refs #8124

* patching expected results refs #21, #8124

* now react to approaching (moving) foes while stuck on junction after green phase ends. refs #8124

* patching expected results refs #21, #8124

* added test refs #8124

* documented new method parameter refs #2

* added new tool refs #10642

* added test refs #10643, #21

* fix #10643

* using file name as default id when loading multiple files. refs  #10643

* moved tool refs #10642

* error handling refs #10642

* added test refs #10642, #21

* #2 doc #10642

* #2 attributeStats with multiple input files

* #2 updated FAQ #10642, #10643

* typo

* patching expected results refs #21

* #2 Changelog

* adapting meso tests #21

* refactoring workflows #12

* adapting meso tests #21

* trivial projection autodetection #10646

* adapting tests #21 #10646

* running astyle #22

* Now Netedit are automatic rerunned if it fails. Refs #3045

* Fixed #10649

* Fixed #10624

* addendum refs #10624

* fix #10647

* Minor update in tractionSubstationType. Refs #457

* Added new NETEDIT test. Refs #3045

* fixing warning #3

* Removed invalid test. Refs #3045

* fixing style #22

* updating docs for command line options #2 #563

* updating xsd schemata #2 #563

* version bump 1.13.0 #563

* #2 Changelog clean-up

* after release cleanup #563

* script to disable screen lock on windows to keep the test server alive #21

* updating open build service link #2

* debian changelog #563 #2

* #2 Changelog: overhead wires were missing

* #2 wording

* handling missing projection info in newer GDALs #10646

* added test refs #10665, #21

* added tests refs #5212

* adding ci tests #21

* moving website update to release day to avoid broken links #2

* #2 typo

* resetting lanechanger state after teleport. refs #10665

* ignoring leaders beyond red light for lane changing. fix #10665

* patching expected results refs #21, #10665

* addendum refs #10665 (sublane model)

* patching expected results refs #21, #10665

* updating network offset and expected shapes. fix #10646, #21

* consistent use of yellow-link coloring. fix #10651

* patching meso results refs #21

* #2 Changelog 1.13.0 leftovers

* can now ignore jammed detectors. refs #5212

* patching expected results refs #21, #5212

* trying to make tests less sensitive to parallel processes #21

* adapting extraction config to different checkout names #21

* can override lane-specific parameters. refs #5212

* added tests refs #21, #5212

* fix #10671

* now indicating jammed detectors (orange,crossed-out). refs #5212

* #2 formatting

* #2 note on opening brackets in xml attributes

* introducing optional inductionLoop length. refs #10668

* alternative drawing style for inductionLoop with length. refs #10668

* #2 doc #5212, #10668

* Fixed #9529

* addendum refs #10089

* patching expected results refs #21, #10089

* Added matrix counter in FPS counter. Refs #10425

* patching expected results refs #21, #10089

* translating from spanisch 'requiere' to english 'require'. refs #12

* Added vertexCounter. Refs #10425

* Added define for enable or disable count vertex and matrix. Refs #10425

* Fix unclosed socket in TraCI `Connection` initializer (#10670)

* docs.python.org/3.5/library/socket.html#example

* Update tools/traci/connection.py

Co-authored-by: Tucker Alban <mtuck2005@yahoo.ca>

Co-authored-by: Michael Behrisch <oss@behrisch.de>

* trying to fix clang13 warnings #3

* tweaking debug code

* added test refs #10676, #21

* fix #10676

* re-organized actuated tests with default branching logic. refs #21

* re-organized tls tests (this suite has become too big to qualify as 'basic') refs #21

* added test refs #10668, #21

* #2 doc #10668

* #2 Changelog

* fixing route file encoding #22

* Added regenerate button. Refs #9072

* Updated GNEJunction. Refs #9072

* Updated GNEChange_TLS. Refs #9072

* Minor update in GNETLSEditorFrame. Refs #9072

* Updated PersonPlans. Refs #10607

* Fixed #10607

* Fixed #7142

* #2 small wording corrections

* update to 1.13

* renamed test input refs #21

* added test refs #9280, #5212, #21

* jam-detection makes all detectors usable. refs #5212, fix #9280

* patching expected results refs #21, #9280

* make 'period' main attribute and allow 'freq' as alias; refs #10390

* update test results; refs #21 #10390

* delete tests

* update readme

* fix merge 1.13

* keep updating 1.13

* trying to speed up appveyor #10689

* using proper warnings fix 10666

* keeping up with upstream 1.13

Signed-off-by: ewht <thoweb@arcor.de>

* friction 1.13

Signed-off-by: ewht <thoweb@arcor.de>

* netgen&netcon meta tests friction

* fixing tests

Signed-off-by: ewht <thoweb@arcor.de>

* fixing tests

* fixing tests

* main GUISUMOVIiesParent

* keep updating 1.13

Signed-off-by: ewht <thoweb@arcor.de>

* updating to 1.13

* merge 1.13

* merge 1.13

* merge 1.13

Co-authored-by: namdre <jakob.erdmann@dlr.de>
Co-authored-by: Pablo Alvarez Lopez <pablo.alvarezlopez@dlr.de>
Co-authored-by: Michael Behrisch <michael.behrisch@dlr.de>
Co-authored-by: Max Schrader <mcschrader@crimson.ua.edu>
Co-authored-by: Angelo Banse <angelo_22@outlook.com>
Co-authored-by: Tucker Alban <mtuck2005@yahoo.ca>
Co-authored-by: Michael Behrisch <oss@behrisch.de>
Co-authored-by: Matthias Schwamborn <matthias.schwamborn@dlr.de>
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 this pull request may close these issues.

None yet

2 participants