4 ms·

Best line of the post: > And as it is with these types of bugs, finding the root cause is not trivial, but the fix almost always is.

xandrius 2y ago
Getting paid not to turn a screw but to know which screw to turn.
rob74 2y ago
This is actually the best case scenario: putting in some effort to find the root cause, having maybe a bit of luck and actually finding it, and then making a minimal change to fix it (hopefully) once and for all. Much better than the typical back-and-forth trial-and-error "I changed something, see if it works now" - which I suspect are much more common in the real world (especially if the reporter is a different person than the one fixing it), but are nothing worth blogging about...
- ralferoo 2y ago
  Well, except that his fix can't easily be pushed back into the source tree because he doesn't know what other software is only working because of the original fix. Presumably it must have been needed by at least one other person, probably also after a crazy long debugging session, or it wouldn't have been there at all. As this code is in a public repo for the default library of a well-used language, it's pretty much a given that fixing this by just removing those lines will cause a regression for someone.
  - pixl97 2y ago
    Heh, which is why you document the code when adding hacks like this in the first place... >#Adding this line to prevent hangs on XXX terminal Or whatever.
    - OJFord 2y ago
      There was in this case, just above the offending lines, including this gem: > [...] This does NOT appoear [sic] to happen with TCP sockets, but this doesn't hurt the TCP side in any case. Oops!
  - gjm11 2y ago
    Unless I'm misunderstanding the post, it's the author's own code. (Here's the relevant change, removing those four lines of code: https://github.com/spc476/lua-conmanorg/commit/0ba8d616d14477603b33fd8f519d9a7b445235bd https://github.com/spc476/lua-conmanorg/commit/0ba8d616d1447...)
    - ralferoo 2y ago
      Oh good spot. I'd misread the repo name as lua-common, I guess my brain "autocorrected" it. The way the article was written, I didn't at all get the impression that it was his code, just that he discovered it and tried hacking it out: > This bit of code wasn't in the TLS implementation, and as the comment says, it “shouldn't” hurt the TCP path, but hey, it was only needed for local (or Unix) domain sockets to begin with, so let me try removing that bit of code and run a test. But yeah, if it's his own project, it's far easier to keep track of and watch out for any bug reports that might result from this change.
  - stanleydrew 2y ago
    The fix doesn't require removing code though as far as I can tell. Could just add an if statement.

xandrius 2y ago

Getting paid not to turn a screw but to know which screw to turn.

rob74 2y ago

This is actually the best case scenario: putting in some effort to find the root cause, having maybe a bit of luck and actually finding it, and then making a minimal change to fix it (hopefully) once and for all. Much better than the typical back-and-forth trial-and-error "I changed something, see if it works now" - which I suspect are much more common in the real world (especially if the reporter is a different person than the one fixing it), but are nothing worth blogging about...

ralferoo 2y ago
Well, except that his fix can't easily be pushed back into the source tree because he doesn't know what other software is only working because of the original fix. Presumably it must have been needed by at least one other person, probably also after a crazy long debugging session, or it wouldn't have been there at all. As this code is in a public repo for the default library of a well-used language, it's pretty much a given that fixing this by just removing those lines will cause a regression for someone.
- pixl97 2y ago
  Heh, which is why you document the code when adding hacks like this in the first place... >#Adding this line to prevent hangs on XXX terminal Or whatever.
  - OJFord 2y ago
    There was in this case, just above the offending lines, including this gem: > [...] This does NOT appoear [sic] to happen with TCP sockets, but this doesn't hurt the TCP side in any case. Oops!
- gjm11 2y ago
  Unless I'm misunderstanding the post, it's the author's own code. (Here's the relevant change, removing those four lines of code: https://github.com/spc476/lua-conmanorg/commit/0ba8d616d14477603b33fd8f519d9a7b445235bd https://github.com/spc476/lua-conmanorg/commit/0ba8d616d1447...)
  - ralferoo 2y ago
    Oh good spot. I'd misread the repo name as lua-common, I guess my brain "autocorrected" it. The way the article was written, I didn't at all get the impression that it was his code, just that he discovered it and tried hacking it out: > This bit of code wasn't in the TLS implementation, and as the comment says, it “shouldn't” hurt the TCP path, but hey, it was only needed for local (or Unix) domain sockets to begin with, so let me try removing that bit of code and run a test. But yeah, if it's his own project, it's far easier to keep track of and watch out for any bug reports that might result from this change.
- stanleydrew 2y ago
  The fix doesn't require removing code though as far as I can tell. Could just add an if statement.