Describe the bug
BiDi.SetLine passes ICU the wrong kind of argument for the line object.
ICU's C signature is:
void ubidi_setLine(const UBiDi *pParaBiDi, int32_t start, int32_t limit,
UBiDi *pLineBiDi, UErrorCode *pErrorCode);pLineBiDi is an input. The caller opens it first with ubidi_open() or ubidi_openSized(). ICU then fills in that object.
icu-dotnet declares it as out IntPtr lineBiDi instead:
So ICU gets the address of an 8-byte managed slot and treats it as a UBiDi struct. It writes a whole struct there, which corrupts memory. The returned BiDi then wraps whatever pointer-sized value ended up in the slot. When that object is disposed or finalized, ubidi_close runs on that value.
BiDi.SetLine has no test, which is probably why this hasn't come up.
Expected behavior
SetLine returns a working line BiDi, and closing it doesn't touch memory that ICU doesn't own.
Possible fix
- Change the parameter to a plain input handle.
- In
SetLine, open the line object first (ubidi_open()), then pass it to ubidi_setLine.
- Keep the paragraph object alive while the line object is in use. ICU's line object points into the paragraph's data.
- Add tests for
SetLine.
ICU also says the paragraph object must not be modified while a line object uses it. Calling SetPara again on the paragraph would leave the line reading stale data. The fix should guard against that, e.g. by making the line throw ObjectDisposedException afterwards.
This isn't a breaking change. The public SetLine(int start, int limit) signature stays the same, and the binding is internal. Today's SetLine can't work, so no caller relies on its current behavior.
Describe the bug
BiDi.SetLinepasses ICU the wrong kind of argument for the line object.ICU's C signature is:
pLineBiDiis an input. The caller opens it first withubidi_open()orubidi_openSized(). ICU then fills in that object.icu-dotnet declares it as
out IntPtr lineBiDiinstead:So ICU gets the address of an 8-byte managed slot and treats it as a
UBiDistruct. It writes a whole struct there, which corrupts memory. The returnedBiDithen wraps whatever pointer-sized value ended up in the slot. When that object is disposed or finalized,ubidi_closeruns on that value.BiDi.SetLinehas no test, which is probably why this hasn't come up.Expected behavior
SetLinereturns a working lineBiDi, and closing it doesn't touch memory that ICU doesn't own.Possible fix
SetLine, open the line object first (ubidi_open()), then pass it toubidi_setLine.SetLine.ICU also says the paragraph object must not be modified while a line object uses it. Calling
SetParaagain on the paragraph would leave the line reading stale data. The fix should guard against that, e.g. by making the line throwObjectDisposedExceptionafterwards.This isn't a breaking change. The public
SetLine(int start, int limit)signature stays the same, and the binding is internal. Today'sSetLinecan't work, so no caller relies on its current behavior.